From efcdb95fa7cf2da7d599e0bcda250514ac5ee048 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 14 Apr 2021 17:31:37 -0700 Subject: [PATCH] Aapt2 ValueTransformer For future macro support, aapt2 must be able to convert Reference values into other Value types. Currently a DescendingValueVisitor is used to visit all of the References in a ResourceTable or a compiled XML file to set their resource ids during the link phase. This was fine since we were only mutating the resource id of the visited Reference. A macro may reference a String, BinaryPrimitive, or any other Item type. During the link phase, we will need to transform references to macros into the values of the macros. The only parameter in the methods of the ValueVisitor interface is a raw pointer to the type being visited. The visitor interface does not support reassigning the visited type to a different type. ValueTransformer is a new interface for consuming a Value type and transforming it into a compatible Value type. This change refactors Value::Clone to use this interface. Bug: 175616308 Test: aapt2_tests Change-Id: Ic1b9d718b932c208764114cd9c74d880e189ccb0 --- tools/aapt2/Android.bp | 1 + tools/aapt2/ResourceTable.cpp | 3 +- tools/aapt2/ResourceValues.cpp | 199 ++++++++++-------- tools/aapt2/ResourceValues.h | 71 ++++--- tools/aapt2/ResourceValues_test.cpp | 12 +- tools/aapt2/ValueTransformer.cpp | 50 +++++ tools/aapt2/ValueTransformer.h | 128 +++++++++++ tools/aapt2/ValueTransformer_inline.h | 47 +++++ tools/aapt2/cmd/Link.cpp | 5 +- tools/aapt2/compile/PseudolocaleGenerator.cpp | 3 +- .../format/binary/TableFlattener_test.cpp | 5 +- tools/aapt2/java/JavaClassGenerator_test.cpp | 6 +- tools/aapt2/link/AutoVersioner.cpp | 3 +- tools/aapt2/link/TableMerger.cpp | 11 +- tools/aapt2/link/XmlCompatVersioner.cpp | 9 +- tools/aapt2/split/TableSplitter.cpp | 4 +- tools/aapt2/test/Context.h | 3 +- tools/aapt2/xml/XmlDom.cpp | 3 +- 18 files changed, 420 insertions(+), 143 deletions(-) create mode 100644 tools/aapt2/ValueTransformer.cpp create mode 100644 tools/aapt2/ValueTransformer.h create mode 100644 tools/aapt2/ValueTransformer_inline.h diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index 3937cee99e3b4..12dc156f75be9 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -162,6 +162,7 @@ cc_library_host_static { "Configuration.proto", "Resources.proto", "ResourcesInternal.proto", + "ValueTransformer.cpp", ], proto: { export_proto_headers: true, diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 27f7bdd83c9e9..45ea65430bb68 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -577,6 +577,7 @@ Maybe ResourceTable::FindResource(const ResourceNam std::unique_ptr ResourceTable::Clone() const { std::unique_ptr new_table = util::make_unique(); + CloningValueTransformer cloner(&new_table->string_pool); for (const auto& pkg : packages) { ResourceTablePackage* new_pkg = new_table->FindOrCreatePackage(pkg->name); for (const auto& type : pkg->types) { @@ -593,7 +594,7 @@ std::unique_ptr ResourceTable::Clone() const { for (const auto& config_value : entry->values) { ResourceConfigValue* new_value = new_entry->FindOrCreateValue(config_value->config, config_value->product); - new_value->value.reset(config_value->value->Clone(&new_table->string_pool)); + new_value->value = config_value->value->Transform(cloner); } } } diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 4f0fa8ae29ba2..574bd2e44a84e 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -47,6 +47,14 @@ std::ostream& operator<<(std::ostream& out, const Value& value) { return out; } +std::unique_ptr Value::Transform(ValueTransformer& transformer) const { + return std::unique_ptr(this->TransformValueImpl(transformer)); +} + +std::unique_ptr Item::Transform(ValueTransformer& transformer) const { + return std::unique_ptr(this->TransformItemImpl(transformer)); +} + template void BaseValue::Accept(ValueVisitor* visitor) { visitor->Visit(static_cast(this)); @@ -77,13 +85,6 @@ bool RawString::Equals(const Value* value) const { return *this->value == *other->value; } -RawString* RawString::Clone(StringPool* new_pool) const { - RawString* rs = new RawString(new_pool->MakeRef(value)); - rs->comment_ = comment_; - rs->source_ = source_; - return rs; -} - bool RawString::Flatten(android::Res_value* out_value) const { out_value->dataType = android::Res_value::TYPE_STRING; out_value->data = util::HostToDevice32(static_cast(value.index())); @@ -136,10 +137,6 @@ bool Reference::Flatten(android::Res_value* out_value) const { return true; } -Reference* Reference::Clone(StringPool* /*new_pool*/) const { - return new Reference(*this); -} - void Reference::Print(std::ostream* out) const { if (reference_type == Type::kResource) { *out << "(reference) @"; @@ -220,10 +217,6 @@ bool Id::Flatten(android::Res_value* out) const { return true; } -Id* Id::Clone(StringPool* /*new_pool*/) const { - return new Id(*this); -} - void Id::Print(std::ostream* out) const { *out << "(id)"; } @@ -266,14 +259,6 @@ bool String::Flatten(android::Res_value* out_value) const { return true; } -String* String::Clone(StringPool* new_pool) const { - String* str = new String(new_pool->MakeRef(value)); - str->comment_ = comment_; - str->source_ = source_; - str->untranslatable_sections = untranslatable_sections; - return str; -} - void String::Print(std::ostream* out) const { *out << "(string) \"" << *value << "\""; } @@ -321,14 +306,6 @@ bool StyledString::Flatten(android::Res_value* out_value) const { return true; } -StyledString* StyledString::Clone(StringPool* new_pool) const { - StyledString* str = new StyledString(new_pool->MakeRef(value)); - str->comment_ = comment_; - str->source_ = source_; - str->untranslatable_sections = untranslatable_sections; - return str; -} - void StyledString::Print(std::ostream* out) const { *out << "(styled string) \"" << value->value << "\""; for (const StringPool::Span& span : value->spans) { @@ -357,15 +334,6 @@ bool FileReference::Flatten(android::Res_value* out_value) const { return true; } -FileReference* FileReference::Clone(StringPool* new_pool) const { - FileReference* fr = new FileReference(new_pool->MakeRef(path)); - fr->file = file; - fr->type = type; - fr->comment_ = comment_; - fr->source_ = source_; - return fr; -} - void FileReference::Print(std::ostream* out) const { *out << "(file) " << *path; switch (type) { @@ -406,10 +374,6 @@ bool BinaryPrimitive::Flatten(::android::Res_value* out_value) const { return true; } -BinaryPrimitive* BinaryPrimitive::Clone(StringPool* /*new_pool*/) const { - return new BinaryPrimitive(*this); -} - void BinaryPrimitive::Print(std::ostream* out) const { *out << StringPrintf("(primitive) type=0x%02x data=0x%08x", value.dataType, value.data); } @@ -587,10 +551,6 @@ bool Attribute::IsCompatibleWith(const Attribute& attr) const { return this_type_mask == that_type_mask; } -Attribute* Attribute::Clone(StringPool* /*new_pool*/) const { - return new Attribute(*this); -} - std::string Attribute::MaskString() const { if (type_mask == android::ResTable_map::TYPE_ANY) { return "any"; @@ -893,18 +853,6 @@ bool Style::Equals(const Value* value) const { }); } -Style* Style::Clone(StringPool* new_pool) const { - Style* style = new Style(); - style->parent = parent; - style->parent_inferred = parent_inferred; - style->comment_ = comment_; - style->source_ = source_; - for (auto& entry : entries) { - style->entries.push_back(Entry{entry.key, std::unique_ptr(entry.value->Clone(new_pool))}); - } - return style; -} - void Style::Print(std::ostream* out) const { *out << "(style) "; if (parent && parent.value().name) { @@ -920,7 +868,8 @@ void Style::Print(std::ostream* out) const { Style::Entry CloneEntry(const Style::Entry& entry, StringPool* pool) { Style::Entry cloned_entry{entry.key}; if (entry.value != nullptr) { - cloned_entry.value.reset(entry.value->Clone(pool)); + CloningValueTransformer cloner(pool); + cloned_entry.value = entry.value->Transform(cloner); } return cloned_entry; } @@ -993,16 +942,6 @@ bool Array::Equals(const Value* value) const { }); } -Array* Array::Clone(StringPool* new_pool) const { - Array* array = new Array(); - array->comment_ = comment_; - array->source_ = source_; - for (auto& item : elements) { - array->elements.emplace_back(std::unique_ptr(item->Clone(new_pool))); - } - return array; -} - void Array::Print(std::ostream* out) const { *out << "(array) [" << util::Joiner(elements, ", ") << "]"; } @@ -1030,19 +969,6 @@ bool Plural::Equals(const Value* value) const { return true; } -Plural* Plural::Clone(StringPool* new_pool) const { - Plural* p = new Plural(); - p->comment_ = comment_; - p->source_ = source_; - const size_t count = values.size(); - for (size_t i = 0; i < count; i++) { - if (values[i]) { - p->values[i] = std::unique_ptr(values[i]->Clone(new_pool)); - } - } - return p; -} - void Plural::Print(std::ostream* out) const { *out << "(plural)"; if (values[Zero]) { @@ -1086,10 +1012,6 @@ bool Styleable::Equals(const Value* value) const { }); } -Styleable* Styleable::Clone(StringPool* /*new_pool*/) const { - return new Styleable(*this); -} - void Styleable::Print(std::ostream* out) const { *out << "(styleable) " << " [" << util::Joiner(entries, ", ") << "]"; @@ -1126,4 +1048,105 @@ void Styleable::MergeWith(Styleable* other) { entries.insert(entries.end(), references.begin(), references.end()); } +template +std::unique_ptr CopyValueFields(std::unique_ptr new_value, const T* value) { + new_value->SetSource(value->GetSource()); + new_value->SetComment(value->GetComment()); + return new_value; +} + +CloningValueTransformer::CloningValueTransformer(StringPool* new_pool) + : ValueTransformer(new_pool) { +} + +std::unique_ptr CloningValueTransformer::TransformDerived(const Reference* value) { + return std::make_unique(*value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived(const Id* value) { + return std::make_unique(*value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived(const RawString* value) { + auto new_value = std::make_unique(pool_->MakeRef(value->value)); + return CopyValueFields(std::move(new_value), value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived(const String* value) { + auto new_value = std::make_unique(pool_->MakeRef(value->value)); + new_value->untranslatable_sections = value->untranslatable_sections; + return CopyValueFields(std::move(new_value), value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived(const StyledString* value) { + auto new_value = std::make_unique(pool_->MakeRef(value->value)); + new_value->untranslatable_sections = value->untranslatable_sections; + return CopyValueFields(std::move(new_value), value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived( + const FileReference* value) { + auto new_value = std::make_unique(pool_->MakeRef(value->path)); + new_value->file = value->file; + new_value->type = value->type; + return CopyValueFields(std::move(new_value), value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived( + const BinaryPrimitive* value) { + return std::make_unique(*value); +} + +std::unique_ptr CloningValueTransformer::TransformDerived(const Attribute* value) { + auto new_value = std::make_unique(); + new_value->type_mask = value->type_mask; + new_value->min_int = value->min_int; + new_value->max_int = value->max_int; + for (const Attribute::Symbol& s : value->symbols) { + new_value->symbols.emplace_back(Attribute::Symbol{ + .symbol = *s.symbol.Transform(*this), + .value = s.value, + .type = s.type, + }); + } + return CopyValueFields(std::move(new_value), value); +} + +std::unique_ptr