AAPT2: Separate out the various steps
An early refactor. Some ideas became clearer as
development continued. Now the various phases are much
clearer and more easily reusable.
Also added a ton of tests!
Change-Id: Ic8f0a70c8222370352e63533b329c40457c0903e
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index c93ecc7..a1e7d36 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -15,11 +15,11 @@
*/
#include "ConfigDescription.h"
-#include "Logger.h"
#include "NameMangler.h"
#include "ResourceTable.h"
#include "ResourceValues.h"
-#include "Util.h"
+#include "ValueVisitor.h"
+#include "util/Util.h"
#include <algorithm>
#include <androidfw/ResourceTypes.h>
@@ -37,65 +37,109 @@
return lhs->type < rhs;
}
-static bool lessThanEntry(const std::unique_ptr<ResourceEntry>& lhs, const StringPiece16& rhs) {
+template <typename T>
+static bool lessThanStructWithName(const std::unique_ptr<T>& lhs,
+ const StringPiece16& rhs) {
return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0;
}
-ResourceTable::ResourceTable() : mPackageId(kUnsetPackageId) {
- // Make sure attrs always have type ID 1.
- findOrCreateType(ResourceType::kAttr)->typeId = 1;
+ResourceTablePackage* ResourceTable::findPackage(const StringPiece16& name) {
+ const auto last = packages.end();
+ auto iter = std::lower_bound(packages.begin(), last, name,
+ lessThanStructWithName<ResourceTablePackage>);
+ if (iter != last && name == (*iter)->name) {
+ return iter->get();
+ }
+ return nullptr;
}
-std::unique_ptr<ResourceTableType>& ResourceTable::findOrCreateType(ResourceType type) {
- auto last = mTypes.end();
- auto iter = std::lower_bound(mTypes.begin(), last, type, lessThanType);
- if (iter != last) {
- if ((*iter)->type == type) {
- return *iter;
+ResourceTablePackage* ResourceTable::findPackageById(uint8_t id) {
+ for (auto& package : packages) {
+ if (package->id && package->id.value() == id) {
+ return package.get();
}
}
- return *mTypes.emplace(iter, new ResourceTableType{ type });
+ return nullptr;
}
-std::unique_ptr<ResourceEntry>& ResourceTable::findOrCreateEntry(
- std::unique_ptr<ResourceTableType>& type, const StringPiece16& name) {
- auto last = type->entries.end();
- auto iter = std::lower_bound(type->entries.begin(), last, name, lessThanEntry);
- if (iter != last) {
- if (name == (*iter)->name) {
- return *iter;
- }
+ResourceTablePackage* ResourceTable::createPackage(const StringPiece16& name, uint8_t id) {
+ ResourceTablePackage* package = findOrCreatePackage(name);
+ if (!package->id) {
+ package->id = id;
+ return package;
}
- return *type->entries.emplace(iter, new ResourceEntry{ name });
+
+ if (package->id.value() == id) {
+ return package;
+ }
+ return nullptr;
}
-struct IsAttributeVisitor : ConstValueVisitor {
- bool isAttribute = false;
-
- void visit(const Attribute&, ValueVisitorArgs&) override {
- isAttribute = true;
+ResourceTablePackage* ResourceTable::findOrCreatePackage(const StringPiece16& name) {
+ const auto last = packages.end();
+ auto iter = std::lower_bound(packages.begin(), last, name,
+ lessThanStructWithName<ResourceTablePackage>);
+ if (iter != last && name == (*iter)->name) {
+ return iter->get();
}
- operator bool() {
- return isAttribute;
+ std::unique_ptr<ResourceTablePackage> newPackage = util::make_unique<ResourceTablePackage>();
+ newPackage->name = name.toString();
+ return packages.emplace(iter, std::move(newPackage))->get();
+}
+
+ResourceTableType* ResourceTablePackage::findType(ResourceType type) {
+ const auto last = types.end();
+ auto iter = std::lower_bound(types.begin(), last, type, lessThanType);
+ if (iter != last && (*iter)->type == type) {
+ return iter->get();
}
-};
+ return nullptr;
+}
+
+ResourceTableType* ResourceTablePackage::findOrCreateType(ResourceType type) {
+ const auto last = types.end();
+ auto iter = std::lower_bound(types.begin(), last, type, lessThanType);
+ if (iter != last && (*iter)->type == type) {
+ return iter->get();
+ }
+ return types.emplace(iter, new ResourceTableType{ type })->get();
+}
+
+ResourceEntry* ResourceTableType::findEntry(const StringPiece16& name) {
+ const auto last = entries.end();
+ auto iter = std::lower_bound(entries.begin(), last, name,
+ lessThanStructWithName<ResourceEntry>);
+ if (iter != last && name == (*iter)->name) {
+ return iter->get();
+ }
+ return nullptr;
+}
+
+ResourceEntry* ResourceTableType::findOrCreateEntry(const StringPiece16& name) {
+ auto last = entries.end();
+ auto iter = std::lower_bound(entries.begin(), last, name,
+ lessThanStructWithName<ResourceEntry>);
+ if (iter != last && name == (*iter)->name) {
+ return iter->get();
+ }
+ return entries.emplace(iter, new ResourceEntry{ name })->get();
+}
/**
* The default handler for collisions. A return value of -1 means keep the
* existing value, 0 means fail, and +1 means take the incoming value.
*/
-static int defaultCollisionHandler(const Value& existing, const Value& incoming) {
- IsAttributeVisitor existingIsAttr, incomingIsAttr;
- existing.accept(existingIsAttr, {});
- incoming.accept(incomingIsAttr, {});
+int ResourceTable::resolveValueCollision(Value* existing, Value* incoming) {
+ Attribute* existingAttr = valueCast<Attribute>(existing);
+ Attribute* incomingAttr = valueCast<Attribute>(incoming);
- if (!incomingIsAttr) {
- if (incoming.isWeak()) {
+ if (!incomingAttr) {
+ if (incoming->isWeak()) {
// We're trying to add a weak resource but a resource
// already exists. Keep the existing.
return -1;
- } else if (existing.isWeak()) {
+ } else if (existing->isWeak()) {
// Override the weak resource with the new strong resource.
return 1;
}
@@ -104,8 +148,8 @@
return 0;
}
- if (!existingIsAttr) {
- if (existing.isWeak()) {
+ if (!existingAttr) {
+ if (existing->isWeak()) {
// The existing value is not an attribute and it is weak,
// so take the incoming attribute value.
return 1;
@@ -115,27 +159,27 @@
return 0;
}
+ assert(incomingAttr && existingAttr);
+
//
// Attribute specific handling. At this point we know both
// values are attributes. Since we can declare and define
// attributes all-over, we do special handling to see
// which definition sticks.
//
- const Attribute& existingAttr = static_cast<const Attribute&>(existing);
- const Attribute& incomingAttr = static_cast<const Attribute&>(incoming);
- if (existingAttr.typeMask == incomingAttr.typeMask) {
+ if (existingAttr->typeMask == incomingAttr->typeMask) {
// The two attributes are both DECLs, but they are plain attributes
// with the same formats.
// Keep the strongest one.
- return existingAttr.isWeak() ? 1 : -1;
+ return existingAttr->isWeak() ? 1 : -1;
}
- if (existingAttr.isWeak() && existingAttr.typeMask == android::ResTable_map::TYPE_ANY) {
+ if (existingAttr->isWeak() && existingAttr->typeMask == android::ResTable_map::TYPE_ANY) {
// Any incoming attribute is better than this.
return 1;
}
- if (incomingAttr.isWeak() && incomingAttr.typeMask == android::ResTable_map::TYPE_ANY) {
+ if (incomingAttr->isWeak() && incomingAttr->typeMask == android::ResTable_map::TYPE_ANY) {
// The incoming attribute may be a USE instead of a DECL.
// Keep the existing attribute.
return -1;
@@ -147,180 +191,183 @@
static constexpr const char16_t* kValidNameMangledChars = u"._-$";
bool ResourceTable::addResource(const ResourceNameRef& name, const ConfigDescription& config,
- const SourceLine& source, std::unique_ptr<Value> value) {
- return addResourceImpl(name, ResourceId{}, config, source, std::move(value), kValidNameChars);
+ const Source& source, std::unique_ptr<Value> value,
+ IDiagnostics* diag) {
+ return addResourceImpl(name, ResourceId{}, config, source, std::move(value), kValidNameChars,
+ diag);
}
bool ResourceTable::addResource(const ResourceNameRef& name, const ResourceId resId,
- const ConfigDescription& config, const SourceLine& source,
- std::unique_ptr<Value> value) {
- return addResourceImpl(name, resId, config, source, std::move(value), kValidNameChars);
+ const ConfigDescription& config, const Source& source,
+ std::unique_ptr<Value> value, IDiagnostics* diag) {
+ return addResourceImpl(name, resId, config, source, std::move(value), kValidNameChars, diag);
+}
+
+bool ResourceTable::addFileReference(const ResourceNameRef& name, const ConfigDescription& config,
+ const Source& source, const StringPiece16& path,
+ IDiagnostics* diag) {
+ return addResourceImpl(name, ResourceId{}, config, source,
+ util::make_unique<FileReference>(stringPool.makeRef(path)),
+ kValidNameChars, diag);
}
bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name,
const ConfigDescription& config,
- const SourceLine& source,
- std::unique_ptr<Value> value) {
+ const Source& source,
+ std::unique_ptr<Value> value,
+ IDiagnostics* diag) {
return addResourceImpl(name, ResourceId{}, config, source, std::move(value),
- kValidNameMangledChars);
+ kValidNameMangledChars, diag);
}
bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceId resId,
- const ConfigDescription& config, const SourceLine& source,
- std::unique_ptr<Value> value, const char16_t* validChars) {
- if (!name.package.empty() && name.package != mPackage) {
- Logger::error(source)
- << "resource '"
- << name
- << "' has incompatible package. Must be '"
- << mPackage
- << "'."
- << std::endl;
- return false;
- }
-
+ const ConfigDescription& config, const Source& source,
+ std::unique_ptr<Value> value, const char16_t* validChars,
+ IDiagnostics* diag) {
auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
if (badCharIter != name.entry.end()) {
- Logger::error(source)
- << "resource '"
- << name
- << "' has invalid entry name '"
- << name.entry
- << "'. Invalid character '"
- << StringPiece16(badCharIter, 1)
- << "'."
- << std::endl;
+ diag->error(DiagMessage(source)
+ << "resource '"
+ << name
+ << "' has invalid entry name '"
+ << name.entry
+ << "'. Invalid character '"
+ << StringPiece16(badCharIter, 1)
+ << "'");
return false;
}
- std::unique_ptr<ResourceTableType>& type = findOrCreateType(name.type);
- if (resId.isValid() && type->typeId != ResourceTableType::kUnsetTypeId &&
- type->typeId != resId.typeId()) {
- Logger::error(source)
- << "trying to add resource '"
- << name
- << "' with ID "
- << resId
- << " but type '"
- << type->type
- << "' already has ID "
- << std::hex << type->typeId << std::dec
- << "."
- << std::endl;
+ ResourceTablePackage* package = findOrCreatePackage(name.package);
+ if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
+ diag->error(DiagMessage(source)
+ << "trying to add resource '"
+ << name
+ << "' with ID "
+ << resId
+ << " but package '"
+ << package->name
+ << "' already has ID "
+ << std::hex << (int) package->id.value() << std::dec);
return false;
}
- std::unique_ptr<ResourceEntry>& entry = findOrCreateEntry(type, name.entry);
- if (resId.isValid() && entry->entryId != ResourceEntry::kUnsetEntryId &&
- entry->entryId != resId.entryId()) {
- Logger::error(source)
- << "trying to add resource '"
- << name
- << "' with ID "
- << resId
- << " but resource already has ID "
- << ResourceId(mPackageId, type->typeId, entry->entryId)
- << "."
- << std::endl;
+ ResourceTableType* type = package->findOrCreateType(name.type);
+ if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
+ diag->error(DiagMessage(source)
+ << "trying to add resource '"
+ << name
+ << "' with ID "
+ << resId
+ << " but type '"
+ << type->type
+ << "' already has ID "
+ << std::hex << (int) type->id.value() << std::dec);
return false;
}
- const auto endIter = std::end(entry->values);
- auto iter = std::lower_bound(std::begin(entry->values), endIter, config, compareConfigs);
+ ResourceEntry* entry = type->findOrCreateEntry(name.entry);
+ if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
+ diag->error(DiagMessage(source)
+ << "trying to add resource '"
+ << name
+ << "' with ID "
+ << resId
+ << " but resource already has ID "
+ << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
+ return false;
+ }
+
+ const auto endIter = entry->values.end();
+ auto iter = std::lower_bound(entry->values.begin(), endIter, config, compareConfigs);
if (iter == endIter || iter->config != config) {
// This resource did not exist before, add it.
entry->values.insert(iter, ResourceConfigValue{ config, source, {}, std::move(value) });
} else {
- int collisionResult = defaultCollisionHandler(*iter->value, *value);
+ int collisionResult = resolveValueCollision(iter->value.get(), value.get());
if (collisionResult > 0) {
// Take the incoming value.
*iter = ResourceConfigValue{ config, source, {}, std::move(value) };
} else if (collisionResult == 0) {
- Logger::error(source)
- << "duplicate value for resource '" << name << "' "
- << "with config '" << iter->config << "'."
- << std::endl;
-
- Logger::error(iter->source)
- << "resource previously defined here."
- << std::endl;
+ diag->error(DiagMessage(source)
+ << "duplicate value for resource '" << name << "' "
+ << "with config '" << iter->config << "'");
+ diag->error(DiagMessage(iter->source)
+ << "resource previously defined here");
return false;
}
}
if (resId.isValid()) {
- type->typeId = resId.typeId();
- entry->entryId = resId.entryId();
+ package->id = resId.packageId();
+ type->id = resId.typeId();
+ entry->id = resId.entryId();
}
return true;
}
bool ResourceTable::markPublic(const ResourceNameRef& name, const ResourceId resId,
- const SourceLine& source) {
- return markPublicImpl(name, resId, source, kValidNameChars);
+ const Source& source, IDiagnostics* diag) {
+ return markPublicImpl(name, resId, source, kValidNameChars, diag);
}
bool ResourceTable::markPublicAllowMangled(const ResourceNameRef& name, const ResourceId resId,
- const SourceLine& source) {
- return markPublicImpl(name, resId, source, kValidNameMangledChars);
+ const Source& source, IDiagnostics* diag) {
+ return markPublicImpl(name, resId, source, kValidNameMangledChars, diag);
}
bool ResourceTable::markPublicImpl(const ResourceNameRef& name, const ResourceId resId,
- const SourceLine& source, const char16_t* validChars) {
- if (!name.package.empty() && name.package != mPackage) {
- Logger::error(source)
- << "resource '"
- << name
- << "' has incompatible package. Must be '"
- << mPackage
- << "'."
- << std::endl;
- return false;
- }
-
+ const Source& source, const char16_t* validChars,
+ IDiagnostics* diag) {
auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
if (badCharIter != name.entry.end()) {
- Logger::error(source)
- << "resource '"
- << name
- << "' has invalid entry name '"
- << name.entry
- << "'. Invalid character '"
- << StringPiece16(badCharIter, 1)
- << "'."
- << std::endl;
+ diag->error(DiagMessage(source)
+ << "resource '"
+ << name
+ << "' has invalid entry name '"
+ << name.entry
+ << "'. Invalid character '"
+ << StringPiece16(badCharIter, 1)
+ << "'");
return false;
}
- std::unique_ptr<ResourceTableType>& type = findOrCreateType(name.type);
- if (resId.isValid() && type->typeId != ResourceTableType::kUnsetTypeId &&
- type->typeId != resId.typeId()) {
- Logger::error(source)
- << "trying to make resource '"
- << name
- << "' public with ID "
- << resId
- << " but type '"
- << type->type
- << "' already has ID "
- << std::hex << type->typeId << std::dec
- << "."
- << std::endl;
+ ResourceTablePackage* package = findOrCreatePackage(name.package);
+ if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
+ diag->error(DiagMessage(source)
+ << "trying to add resource '"
+ << name
+ << "' with ID "
+ << resId
+ << " but package '"
+ << package->name
+ << "' already has ID "
+ << std::hex << (int) package->id.value() << std::dec);
return false;
}
- std::unique_ptr<ResourceEntry>& entry = findOrCreateEntry(type, name.entry);
- if (resId.isValid() && entry->entryId != ResourceEntry::kUnsetEntryId &&
- entry->entryId != resId.entryId()) {
- Logger::error(source)
- << "trying to make resource '"
- << name
- << "' public with ID "
- << resId
- << " but resource already has ID "
- << ResourceId(mPackageId, type->typeId, entry->entryId)
- << "."
- << std::endl;
+ ResourceTableType* type = package->findOrCreateType(name.type);
+ if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
+ diag->error(DiagMessage(source)
+ << "trying to add resource '"
+ << name
+ << "' with ID "
+ << resId
+ << " but type '"
+ << type->type
+ << "' already has ID "
+ << std::hex << (int) type->id.value() << std::dec);
+ return false;
+ }
+
+ ResourceEntry* entry = type->findOrCreateEntry(name.entry);
+ if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
+ diag->error(DiagMessage(source)
+ << "trying to add resource '"
+ << name
+ << "' with ID "
+ << resId
+ << " but resource already has ID "
+ << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
return false;
}
@@ -329,102 +376,30 @@
entry->publicStatus.source = source;
if (resId.isValid()) {
- type->typeId = resId.typeId();
- entry->entryId = resId.entryId();
+ package->id = resId.packageId();
+ type->id = resId.typeId();
+ entry->id = resId.entryId();
}
return true;
}
-bool ResourceTable::merge(ResourceTable&& other) {
- const bool mangleNames = mPackage != other.getPackage();
- std::u16string mangledName;
-
- for (auto& otherType : other) {
- std::unique_ptr<ResourceTableType>& type = findOrCreateType(otherType->type);
- if (otherType->publicStatus.isPublic) {
- if (type->publicStatus.isPublic && type->typeId != otherType->typeId) {
- Logger::error() << "can not merge type '" << type->type
- << "': conflicting public IDs "
- << "(" << type->typeId << " vs " << otherType->typeId << ")."
- << std::endl;
- return false;
- }
- type->publicStatus = std::move(otherType->publicStatus);
- type->typeId = otherType->typeId;
- }
-
- for (auto& otherEntry : otherType->entries) {
- const std::u16string* nameToAdd = &otherEntry->name;
- if (mangleNames) {
- mangledName = otherEntry->name;
- NameMangler::mangle(other.getPackage(), &mangledName);
- nameToAdd = &mangledName;
- }
-
- std::unique_ptr<ResourceEntry>& entry = findOrCreateEntry(type, *nameToAdd);
- if (otherEntry->publicStatus.isPublic) {
- if (entry->publicStatus.isPublic && entry->entryId != otherEntry->entryId) {
- Logger::error() << "can not merge entry '" << type->type << "/" << entry->name
- << "': conflicting public IDs "
- << "(" << entry->entryId << " vs " << entry->entryId << ")."
- << std::endl;
- return false;
- }
- entry->publicStatus = std::move(otherEntry->publicStatus);
- entry->entryId = otherEntry->entryId;
- }
-
- for (ResourceConfigValue& otherValue : otherEntry->values) {
- auto iter = std::lower_bound(entry->values.begin(), entry->values.end(),
- otherValue.config, compareConfigs);
- if (iter != entry->values.end() && iter->config == otherValue.config) {
- int collisionResult = defaultCollisionHandler(*iter->value, *otherValue.value);
- if (collisionResult > 0) {
- // Take the incoming value.
- iter->source = std::move(otherValue.source);
- iter->comment = std::move(otherValue.comment);
- iter->value = std::unique_ptr<Value>(otherValue.value->clone(&mValuePool));
- } else if (collisionResult == 0) {
- ResourceNameRef resourceName = { mPackage, type->type, entry->name };
- Logger::error(otherValue.source)
- << "resource '" << resourceName << "' has a conflicting value for "
- << "configuration (" << otherValue.config << ")."
- << std::endl;
- Logger::note(iter->source) << "originally defined here." << std::endl;
- return false;
- }
- } else {
- entry->values.insert(iter, ResourceConfigValue{
- otherValue.config,
- std::move(otherValue.source),
- std::move(otherValue.comment),
- std::unique_ptr<Value>(otherValue.value->clone(&mValuePool)),
- });
- }
- }
- }
- }
- return true;
-}
-
-std::tuple<const ResourceTableType*, const ResourceEntry*>
-ResourceTable::findResource(const ResourceNameRef& name) const {
- if (name.package != mPackage) {
+Maybe<ResourceTable::SearchResult>
+ResourceTable::findResource(const ResourceNameRef& name) {
+ ResourceTablePackage* package = findPackage(name.package);
+ if (!package) {
return {};
}
- auto iter = std::lower_bound(mTypes.begin(), mTypes.end(), name.type, lessThanType);
- if (iter == mTypes.end() || (*iter)->type != name.type) {
+ ResourceTableType* type = package->findType(name.type);
+ if (!type) {
return {};
}
- const std::unique_ptr<ResourceTableType>& type = *iter;
- auto iter2 = std::lower_bound(type->entries.begin(), type->entries.end(), name.entry,
- lessThanEntry);
- if (iter2 == type->entries.end() || name.entry != (*iter2)->name) {
+ ResourceEntry* entry = type->findEntry(name.entry);
+ if (!entry) {
return {};
}
- return std::make_tuple(iter->get(), iter2->get());
+ return SearchResult{ package, type, entry };
}
} // namespace aapt