diff --git a/tools/aapt2/JavaClassGenerator_test.cpp b/tools/aapt2/JavaClassGenerator_test.cpp
index becf99b521ed4..cc5e98150ae3f 100644
--- a/tools/aapt2/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/JavaClassGenerator_test.cpp
@@ -105,11 +105,9 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
             .addSimple(u"@android:id/one", ResourceId(0x01020000))
             .addSimple(u"@android:id/two", ResourceId(0x01020001))
             .addSimple(u"@android:id/three", ResourceId(0x01020002))
+            .setSymbolState(u"@android:id/one", ResourceId(0x01020000), SymbolState::kPublic)
+            .setSymbolState(u"@android:id/two", ResourceId(0x01020001), SymbolState::kPrivate)
             .build();
-    ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:id/one"), {}, {},
-                                      SymbolState::kPublic, &diag));
-    ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:id/two"), {}, {},
-                                      SymbolState::kPrivate, &diag));
 
     JavaClassGeneratorOptions options;
     options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic;
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index bfef9d06d7424..44710ebc9dc44 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -186,6 +186,7 @@ struct ParsedResource {
     Source source;
     ResourceId id;
     SymbolState symbolState = SymbolState::kUndefined;
+    std::u16string comment;
     std::unique_ptr<Value> value;
     std::list<ParsedResource> childResources;
 };
@@ -194,7 +195,11 @@ struct ParsedResource {
 static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& config,
                                 IDiagnostics* diag, ParsedResource* res) {
     if (res->symbolState != SymbolState::kUndefined) {
-        if (!table->setSymbolState(res->name, res->id, res->source, res->symbolState, diag)) {
+        Symbol symbol;
+        symbol.state = res->symbolState;
+        symbol.source = res->source;
+        symbol.comment = res->comment;
+        if (!table->setSymbolState(res->name, res->id, symbol, diag)) {
             return false;
         }
     }
@@ -203,7 +208,11 @@ static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& c
         return true;
     }
 
-    if (!table->addResource(res->name, res->id, config, res->source, std::move(res->value), diag)) {
+    // Attach the comment, source and config to the value.
+    res->value->setComment(std::move(res->comment));
+    res->value->setSource(std::move(res->source));
+
+    if (!table->addResource(res->name, res->id, config, std::move(res->value), diag)) {
         return false;
     }
 
@@ -275,6 +284,7 @@ bool ResourceParser::parseResources(XmlPullParser* parser) {
         ParsedResource parsedResource;
         parsedResource.name.entry = maybeName.value().toString();
         parsedResource.source = mSource.withLine(parser->getLineNumber());
+        parsedResource.comment = std::move(comment);
 
         bool result = true;
         if (elementName == u"id") {
@@ -368,8 +378,8 @@ enum {
  * an Item. If allowRawValue is false, nullptr is returned in this
  * case.
  */
-std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, uint32_t typeMask,
-                                               bool allowRawValue) {
+std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, const uint32_t typeMask,
+                                               const bool allowRawValue) {
     const size_t beginXmlLine = parser->getLineNumber();
 
     std::u16string rawValue;
@@ -386,8 +396,9 @@ std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, uint32_t t
 
     auto onCreateReference = [&](const ResourceName& name) {
         // name.package can be empty here, as it will assume the package name of the table.
-        mTable->addResource(name, {}, mSource.withLine(beginXmlLine), util::make_unique<Id>(),
-                            mDiag);
+        std::unique_ptr<Id> id = util::make_unique<Id>();
+        id->setSource(mSource.withLine(beginXmlLine));
+        mTable->addResource(name, {}, std::move(id), mDiag);
     };
 
     // Process the raw value.
@@ -411,11 +422,12 @@ std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, uint32_t t
                 mTable->stringPool.makeRef(styleString.str, StringPool::Context{ 1, mConfig }));
     }
 
-    // We can't parse this so return a RawString if we are allowed.
     if (allowRawValue) {
+        // We can't parse this so return a RawString if we are allowed.
         return util::make_unique<RawString>(
                 mTable->stringPool.makeRef(rawValue, StringPool::Context{ 1, mConfig }));
     }
+
     return {};
 }
 
@@ -683,8 +695,8 @@ Maybe<Attribute::Symbol> ResourceParser::parseEnumOrFlagItem(XmlPullParser* pars
     }
 
     return Attribute::Symbol{
-        Reference(ResourceName{ {}, ResourceType::kId, maybeName.value().toString() }),
-                val.data };
+            Reference(ResourceName({}, ResourceType::kId, maybeName.value().toString())),
+            val.data };
 }
 
 static Maybe<ResourceName> parseXmlAttributeName(StringPiece16 str) {
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index 34c68d7257a35..2d5a29df2c491 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -65,12 +65,13 @@ private:
                            StyleString* outStyleString);
 
     /*
-     * Parses the XML subtree and converts it to an Item. The type of Item that can be
-     * parsed is denoted by the `typeMask`. If `allowRawValue` is true and the subtree
-     * can not be parsed as a regular Item, then a RawString is returned. Otherwise
-     * this returns nullptr.
+     * Parses the XML subtree and returns an Item.
+     * The type of Item that can be parsed is denoted by the `typeMask`.
+     * If `allowRawValue` is true and the subtree can not be parsed as a regular Item, then a
+     * RawString is returned. Otherwise this returns false;
      */
-    std::unique_ptr<Item> parseXml(XmlPullParser* parser, uint32_t typeMask, bool allowRawValue);
+    std::unique_ptr<Item> parseXml(XmlPullParser* parser, const uint32_t typeMask,
+                                   const bool allowRawValue);
 
     bool parseResources(XmlPullParser* parser);
     bool parseString(XmlPullParser* parser, ParsedResource* outResource);
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index a7e9d390ef4af..af6bf67c4a3b0 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -379,18 +379,39 @@ TEST_F(ResourceParserTest, ParsePlural) {
 }
 
 TEST_F(ResourceParserTest, ParseCommentsWithResource) {
-    std::string input = "<!-- This is a comment -->\n"
+    std::string input = "<!--This is a comment-->\n"
                         "<string name=\"foo\">Hi</string>";
     ASSERT_TRUE(testParse(input));
 
-    Maybe<ResourceTable::SearchResult> result = mTable.findResource(
-            test::parseNameOrDie(u"@string/foo"));
-    AAPT_ASSERT_TRUE(result);
+    String* value = test::getValue<String>(&mTable, u"@string/foo");
+    ASSERT_NE(nullptr, value);
+    EXPECT_EQ(value->getComment(), u"This is a comment");
+}
 
-    ResourceEntry* entry = result.value().entry;
-    ASSERT_NE(entry, nullptr);
-    ASSERT_FALSE(entry->values.empty());
-    EXPECT_EQ(entry->values.front().comment, u"This is a comment");
+TEST_F(ResourceParserTest, DoNotCombineMultipleComments) {
+    std::string input = "<!--One-->\n"
+                        "<!--Two-->\n"
+                        "<string name=\"foo\">Hi</string>";
+
+    ASSERT_TRUE(testParse(input));
+
+    String* value = test::getValue<String>(&mTable, u"@string/foo");
+    ASSERT_NE(nullptr, value);
+    EXPECT_EQ(value->getComment(), u"Two");
+}
+
+TEST_F(ResourceParserTest, IgnoreCommentBeforeEndTag) {
+    std::string input = "<!--One-->\n"
+                        "<string name=\"foo\">\n"
+                        "  Hi\n"
+                        "<!--Two-->\n"
+                        "</string>";
+
+    ASSERT_TRUE(testParse(input));
+
+    String* value = test::getValue<String>(&mTable, u"@string/foo");
+    ASSERT_NE(nullptr, value);
+    EXPECT_EQ(value->getComment(), u"One");
 }
 
 /*
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 84674e841063d..fa4b1094434b7 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -19,6 +19,8 @@
 #include "ResourceTable.h"
 #include "ResourceValues.h"
 #include "ValueVisitor.h"
+
+#include "util/Comparators.h"
 #include "util/Util.h"
 
 #include <algorithm>
@@ -29,10 +31,6 @@
 
 namespace aapt {
 
-static bool compareConfigs(const ResourceConfigValue& lhs, const ConfigDescription& rhs) {
-    return lhs.config < rhs;
-}
-
 static bool lessThanType(const std::unique_ptr<ResourceTableType>& lhs, ResourceType rhs) {
     return lhs->type < rhs;
 }
@@ -191,52 +189,50 @@ static constexpr const char16_t* kValidNameChars = u"._-";
 static constexpr const char16_t* kValidNameMangledChars = u"._-$";
 
 bool ResourceTable::addResource(const ResourceNameRef& name, const ConfigDescription& config,
-                                const Source& source, std::unique_ptr<Value> value,
-                                IDiagnostics* diag) {
-    return addResourceImpl(name, ResourceId{}, config, source, std::move(value), kValidNameChars,
-                           diag);
+                                std::unique_ptr<Value> value, IDiagnostics* diag) {
+    return addResourceImpl(name, {}, config, std::move(value), kValidNameChars, diag);
 }
 
 bool ResourceTable::addResource(const ResourceNameRef& name, const ResourceId resId,
-                                const ConfigDescription& config, const Source& source,
-                                std::unique_ptr<Value> value, IDiagnostics* diag) {
-    return addResourceImpl(name, resId, config, source, std::move(value), kValidNameChars, diag);
+                                const ConfigDescription& config, std::unique_ptr<Value> value,
+                                IDiagnostics* diag) {
+    return addResourceImpl(name, resId, config, 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);
+    std::unique_ptr<FileReference> fileRef = util::make_unique<FileReference>(
+            stringPool.makeRef(path));
+    fileRef->setSource(source);
+    return addResourceImpl(name, ResourceId{}, config, std::move(fileRef), kValidNameChars, diag);
 }
 
 bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name,
                                             const ConfigDescription& config,
-                                            const Source& source,
                                             std::unique_ptr<Value> value,
                                             IDiagnostics* diag) {
-    return addResourceImpl(name, ResourceId{}, config, source, std::move(value),
-                           kValidNameMangledChars, diag);
+    return addResourceImpl(name, ResourceId{}, config, std::move(value), kValidNameMangledChars,
+                           diag);
 }
 
 bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name,
                                             const ResourceId id,
                                             const ConfigDescription& config,
-                                            const Source& source,
                                             std::unique_ptr<Value> value,
                                             IDiagnostics* diag) {
-    return addResourceImpl(name, id, config, source, std::move(value),
-                           kValidNameMangledChars, diag);
+    return addResourceImpl(name, id, config, std::move(value), kValidNameMangledChars, diag);
 }
 
 bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceId resId,
-                                    const ConfigDescription& config, const Source& source,
-                                    std::unique_ptr<Value> value, const char16_t* validChars,
-                                    IDiagnostics* diag) {
+                                    const ConfigDescription& config, std::unique_ptr<Value> value,
+                                    const char16_t* validChars, IDiagnostics* diag) {
+    assert(value && "value can't be nullptr");
+    assert(diag && "diagnostics can't be nullptr");
+
     auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
     if (badCharIter != name.entry.end()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(value->getSource())
                     << "resource '"
                     << name
                     << "' has invalid entry name '"
@@ -249,7 +245,7 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
 
     ResourceTablePackage* package = findOrCreatePackage(name.package);
     if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(value->getSource())
                     << "trying to add resource '"
                     << name
                     << "' with ID "
@@ -263,7 +259,7 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
 
     ResourceTableType* type = package->findOrCreateType(name.type);
     if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(value->getSource())
                     << "trying to add resource '"
                     << name
                     << "' with ID "
@@ -277,7 +273,7 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
 
     ResourceEntry* entry = type->findOrCreateEntry(name.entry);
     if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(value->getSource())
                     << "trying to add resource '"
                     << name
                     << "' with ID "
@@ -288,20 +284,20 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
     }
 
     const auto endIter = entry->values.end();
-    auto iter = std::lower_bound(entry->values.begin(), endIter, config, compareConfigs);
+    auto iter = std::lower_bound(entry->values.begin(), endIter, config, cmp::lessThan);
     if (iter == endIter || iter->config != config) {
         // This resource did not exist before, add it.
-        entry->values.insert(iter, ResourceConfigValue{ config, source, {}, std::move(value) });
+        entry->values.insert(iter, ResourceConfigValue{ config, std::move(value) });
     } else {
         int collisionResult = resolveValueCollision(iter->value.get(), value.get());
         if (collisionResult > 0) {
             // Take the incoming value.
-            *iter = ResourceConfigValue{ config, source, {}, std::move(value) };
+            iter->value = std::move(value);
         } else if (collisionResult == 0) {
-            diag->error(DiagMessage(source)
+            diag->error(DiagMessage(value->getSource())
                         << "duplicate value for resource '" << name << "' "
-                        << "with config '" << iter->config << "'");
-            diag->error(DiagMessage(iter->source)
+                        << "with config '" << config << "'");
+            diag->error(DiagMessage(iter->value->getSource())
                         << "resource previously defined here");
             return false;
         }
@@ -316,27 +312,29 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
 }
 
 bool ResourceTable::setSymbolState(const ResourceNameRef& name, const ResourceId resId,
-                                   const Source& source, SymbolState state, IDiagnostics* diag) {
-    return setSymbolStateImpl(name, resId, source, state, kValidNameChars, diag);
+                                   const Symbol& symbol, IDiagnostics* diag) {
+    return setSymbolStateImpl(name, resId, symbol, kValidNameChars, diag);
 }
 
-bool ResourceTable::setSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId resId,
-                                               const Source& source, SymbolState state,
-                                               IDiagnostics* diag) {
-    return setSymbolStateImpl(name, resId, source, state, kValidNameMangledChars, diag);
+bool ResourceTable::setSymbolStateAllowMangled(const ResourceNameRef& name,
+                                               const ResourceId resId,
+                                               const Symbol& symbol, IDiagnostics* diag) {
+    return setSymbolStateImpl(name, resId, symbol, kValidNameMangledChars, diag);
 }
 
 bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const ResourceId resId,
-                                       const Source& source, SymbolState state,
-                                       const char16_t* validChars, IDiagnostics* diag) {
-    if (state == SymbolState::kUndefined) {
+                                       const Symbol& symbol, const char16_t* validChars,
+                                       IDiagnostics* diag) {
+    assert(diag && "diagnostics can't be nullptr");
+
+    if (symbol.state == SymbolState::kUndefined) {
         // Nothing to do.
         return true;
     }
 
     auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
     if (badCharIter != name.entry.end()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(symbol.source)
                     << "resource '"
                     << name
                     << "' has invalid entry name '"
@@ -349,7 +347,7 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour
 
     ResourceTablePackage* package = findOrCreatePackage(name.package);
     if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(symbol.source)
                     << "trying to add resource '"
                     << name
                     << "' with ID "
@@ -363,7 +361,7 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour
 
     ResourceTableType* type = package->findOrCreateType(name.type);
     if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(symbol.source)
                     << "trying to add resource '"
                     << name
                     << "' with ID "
@@ -377,7 +375,7 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour
 
     ResourceEntry* entry = type->findOrCreateEntry(name.entry);
     if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
-        diag->error(DiagMessage(source)
+        diag->error(DiagMessage(symbol.source)
                     << "trying to add resource '"
                     << name
                     << "' with ID "
@@ -388,15 +386,14 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour
     }
 
     // Only mark the type state as public, it doesn't care about being private.
-    if (state == SymbolState::kPublic) {
+    if (symbol.state == SymbolState::kPublic) {
         type->symbolStatus.state = SymbolState::kPublic;
     }
 
     // Downgrading to a private symbol from a public one is not allowed.
     if (entry->symbolStatus.state != SymbolState::kPublic) {
-        if (entry->symbolStatus.state != state) {
-            entry->symbolStatus.state = state;
-            entry->symbolStatus.source = source;
+        if (entry->symbolStatus.state != symbol.state) {
+            entry->symbolStatus = std::move(symbol);
         }
     }
 
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index be909361bef10..980504be377b2 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -47,12 +47,10 @@ struct Symbol {
 };
 
 /**
- * The resource value for a specific configuration.
+ * Represents a value defined for a given configuration.
  */
 struct ResourceConfigValue {
     ConfigDescription config;
-    Source source;
-    std::u16string comment;
     std::unique_ptr<Value> value;
 };
 
@@ -158,12 +156,11 @@ public:
     static int resolveValueCollision(Value* existing, Value* incoming);
 
     bool addResource(const ResourceNameRef& name, const ConfigDescription& config,
-                     const Source& source, std::unique_ptr<Value> value,
-                     IDiagnostics* diag);
+                     std::unique_ptr<Value> value, IDiagnostics* diag);
 
     bool addResource(const ResourceNameRef& name, const ResourceId resId,
-                     const ConfigDescription& config, const Source& source,
-                     std::unique_ptr<Value> value, IDiagnostics* diag);
+                     const ConfigDescription& config, std::unique_ptr<Value> value,
+                     IDiagnostics* diag);
 
     bool addFileReference(const ResourceNameRef& name, const ConfigDescription& config,
                           const Source& source, const StringPiece16& path, IDiagnostics* diag);
@@ -174,18 +171,18 @@ public:
      * names.
      */
     bool addResourceAllowMangled(const ResourceNameRef& name, const ConfigDescription& config,
-                                 const Source& source, std::unique_ptr<Value> value,
-                                 IDiagnostics* diag);
+                                 std::unique_ptr<Value> value, IDiagnostics* diag);
 
     bool addResourceAllowMangled(const ResourceNameRef& name, const ResourceId id,
-                                 const ConfigDescription& config,
-                                 const Source& source, std::unique_ptr<Value> value,
+                                 const ConfigDescription& config, std::unique_ptr<Value> value,
                                  IDiagnostics* diag);
 
-    bool setSymbolState(const ResourceNameRef& name, const ResourceId resId, const Source& source,
-                        SymbolState state, IDiagnostics* diag);
+    bool setSymbolState(const ResourceNameRef& name, const ResourceId resId,
+                        const Symbol& symbol, IDiagnostics* diag);
+
     bool setSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId resId,
-                                    const Source& source, SymbolState state, IDiagnostics* diag);
+                                    const Symbol& symbol, IDiagnostics* diag);
+
     struct SearchResult {
         ResourceTablePackage* package;
         ResourceTableType* type;
@@ -224,13 +221,11 @@ public:
 private:
     ResourceTablePackage* findOrCreatePackage(const StringPiece16& name);
 
-    bool addResourceImpl(const ResourceNameRef& name, const ResourceId resId,
-                         const ConfigDescription& config, const Source& source,
-                         std::unique_ptr<Value> value, const char16_t* validChars,
-                         IDiagnostics* diag);
-    bool setSymbolStateImpl(const ResourceNameRef& name, const ResourceId resId,
-                            const Source& source, SymbolState state, const char16_t* validChars,
-                            IDiagnostics* diag);
+    bool addResourceImpl(const ResourceNameRef& name, ResourceId resId,
+                         const ConfigDescription& config, std::unique_ptr<Value> value,
+                         const char16_t* validChars, IDiagnostics* diag);
+    bool setSymbolStateImpl(const ResourceNameRef& name, ResourceId resId,
+                            const Symbol& symbol, const char16_t* validChars, IDiagnostics* diag);
 };
 
 } // namespace aapt
diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp
index 2055a80aaba6c..42508fe154b8d 100644
--- a/tools/aapt2/ResourceTable_test.cpp
+++ b/tools/aapt2/ResourceTable_test.cpp
@@ -19,7 +19,7 @@
 #include "ResourceValues.h"
 #include "util/Util.h"
 
-#include "test/Common.h"
+#include "test/Builders.h"
 
 #include <algorithm>
 #include <gtest/gtest.h>
@@ -42,22 +42,26 @@ TEST_F(ResourceTableTest, FailToAddResourceWithBadName) {
     ResourceTable table;
 
     EXPECT_FALSE(table.addResource(
-            ResourceNameRef{ u"android", ResourceType::kId, u"hey,there" },
-            {}, Source{ "test.xml", 21 },
-            util::make_unique<Id>(), &mDiagnostics));
+            ResourceNameRef(u"android", ResourceType::kId, u"hey,there"),
+            ConfigDescription{},
+            test::ValueBuilder<Id>().setSource("test.xml", 21u).build(),
+            &mDiagnostics));
 
     EXPECT_FALSE(table.addResource(
-            ResourceNameRef{ u"android", ResourceType::kId, u"hey:there" },
-            {}, Source{ "test.xml", 21 },
-            util::make_unique<Id>(), &mDiagnostics));
+            ResourceNameRef(u"android", ResourceType::kId, u"hey:there"),
+            ConfigDescription{},
+            test::ValueBuilder<Id>().setSource("test.xml", 21u).build(),
+            &mDiagnostics));
 }
 
 TEST_F(ResourceTableTest, AddOneResource) {
     ResourceTable table;
 
-    EXPECT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/id"), {},
-                                  Source{ "test/path/file.xml", 23 },
-                                  util::make_unique<Id>(), &mDiagnostics));
+    EXPECT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/id"),
+                                  ConfigDescription{},
+                                  test::ValueBuilder<Id>()
+                                          .setSource("test/path/file.xml", 23u).build(),
+                                  &mDiagnostics));
 
     ASSERT_NE(nullptr, test::getValue<Id>(&table, u"@android:attr/id"));
 }
@@ -71,23 +75,29 @@ TEST_F(ResourceTableTest, AddMultipleResources) {
 
     EXPECT_TRUE(table.addResource(
             test::parseNameOrDie(u"@android:attr/layout_width"),
-            config, Source{ "test/path/file.xml", 10 },
-            util::make_unique<Id>(), &mDiagnostics));
+            config,
+            test::ValueBuilder<Id>().setSource("test/path/file.xml", 10u).build(),
+            &mDiagnostics));
 
     EXPECT_TRUE(table.addResource(
             test::parseNameOrDie(u"@android:attr/id"),
-            config, Source{ "test/path/file.xml", 12 },
-            util::make_unique<Id>(), &mDiagnostics));
+            config,
+            test::ValueBuilder<Id>().setSource("test/path/file.xml", 12u).build(),
+            &mDiagnostics));
 
     EXPECT_TRUE(table.addResource(
             test::parseNameOrDie(u"@android:string/ok"),
-            config, Source{ "test/path/file.xml", 14 },
-            util::make_unique<Id>(), &mDiagnostics));
+            config,
+            test::ValueBuilder<Id>().setSource("test/path/file.xml", 14u).build(),
+            &mDiagnostics));
 
     EXPECT_TRUE(table.addResource(
             test::parseNameOrDie(u"@android:string/ok"),
-            languageConfig, Source{ "test/path/file.xml", 20 },
-            util::make_unique<BinaryPrimitive>(android::Res_value{}), &mDiagnostics));
+            languageConfig,
+            test::ValueBuilder<BinaryPrimitive>(android::Res_value{})
+                    .setSource("test/path/file.xml", 20u)
+                    .build(),
+            &mDiagnostics));
 
     ASSERT_NE(nullptr, test::getValue<Id>(&table, u"@android:attr/layout_width"));
     ASSERT_NE(nullptr, test::getValue<Id>(&table, u"@android:attr/id"));
@@ -99,14 +109,14 @@ TEST_F(ResourceTableTest, AddMultipleResources) {
 TEST_F(ResourceTableTest, OverrideWeakResourceValue) {
     ResourceTable table;
 
-    ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/foo"), {}, {},
+    ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/foo"), ConfigDescription{},
                                   util::make_unique<Attribute>(true), &mDiagnostics));
 
     Attribute* attr = test::getValue<Attribute>(&table, u"@android:attr/foo");
     ASSERT_NE(nullptr, attr);
     EXPECT_TRUE(attr->isWeak());
 
-    ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/foo"), {}, {},
+    ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/foo"), ConfigDescription{},
                                   util::make_unique<Attribute>(false), &mDiagnostics));
 
     attr = test::getValue<Attribute>(&table, u"@android:attr/foo");
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index ecc5cd2bdcfab..f312d75ab1e9b 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -15,11 +15,12 @@
  */
 
 #include "Resource.h"
-#include "flatten/ResourceTypeExtensions.h"
 #include "ResourceValues.h"
-#include "util/Util.h"
 #include "ValueVisitor.h"
 
+#include "util/Util.h"
+#include "flatten/ResourceTypeExtensions.h"
+
 #include <androidfw/ResourceTypes.h>
 #include <limits>
 
@@ -35,18 +36,10 @@ void BaseItem<Derived>::accept(RawValueVisitor* visitor) {
     visitor->visit(static_cast<Derived*>(this));
 }
 
-bool Value::isItem() const {
-    return false;
-}
-
 bool Value::isWeak() const {
     return false;
 }
 
-bool Item::isItem() const {
-    return true;
-}
-
 RawString::RawString(const StringPool::Ref& ref) : value(ref) {
 }
 
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 0dae091dffb43..26291536ae80f 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -40,17 +40,42 @@ struct RawValueVisitor;
 struct Value {
 	virtual ~Value() = default;
 
-    /**
-     * Whether or not this is an Item.
-     */
-    virtual bool isItem() const;
-
     /**
      * Whether this value is weak and can be overridden without
      * warning or error. Default for base class is false.
      */
     virtual bool isWeak() const;
 
+    /**
+     * Returns the source where this value was defined.
+     */
+    const Source& getSource() const {
+        return mSource;
+    }
+
+    void setSource(const Source& source) {
+        mSource = source;
+    }
+
+    void setSource(Source&& source) {
+        mSource = std::move(source);
+    }
+
+    /**
+     * Returns the comment that was associated with this resource.
+     */
+    StringPiece16 getComment() const {
+        return mComment;
+    }
+
+    void setComment(const StringPiece16& str) {
+        mComment = str.toString();
+    }
+
+    void setComment(std::u16string&& str) {
+        mComment = std::move(str);
+    }
+
     /**
      * Calls the appropriate overload of ValueVisitor.
      */
@@ -65,6 +90,10 @@ struct Value {
      * Human readable printout of this value.
      */
     virtual void print(std::ostream* out) const = 0;
+
+private:
+    Source mSource;
+    std::u16string mComment;
 };
 
 /**
@@ -79,11 +108,6 @@ struct BaseValue : public Value {
  * A resource item with a single value. This maps to android::ResTable_entry.
  */
 struct Item : public Value {
-    /**
-     * An Item is, of course, an Item.
-     */
-    virtual bool isItem() const override;
-
     /**
      * Clone the Item.
      */
diff --git a/tools/aapt2/ValueVisitor.h b/tools/aapt2/ValueVisitor.h
index ee058aa1a37b6..94042e3c2618c 100644
--- a/tools/aapt2/ValueVisitor.h
+++ b/tools/aapt2/ValueVisitor.h
@@ -114,6 +114,18 @@ struct DynCastVisitor : public RawValueVisitor {
     }
 };
 
+/**
+ * Specialization that checks if the value is an Item.
+ */
+template <>
+struct DynCastVisitor<Item> : public RawValueVisitor {
+    Item* value = nullptr;
+
+    void visitItem(Item* item) override {
+        value = item;
+    }
+};
+
 /**
  * Returns a valid pointer to T if the Value is of subtype T.
  * Otherwise, returns nullptr.
diff --git a/tools/aapt2/XmlPullParser.cpp b/tools/aapt2/XmlPullParser.cpp
index 1b9499d78efb9..cff935c10aaef 100644
--- a/tools/aapt2/XmlPullParser.cpp
+++ b/tools/aapt2/XmlPullParser.cpp
@@ -97,7 +97,7 @@ const std::string& XmlPullParser::getLastError() const {
 }
 
 const std::u16string& XmlPullParser::getComment() const {
-    return mEventQueue.front().comment;
+    return mEventQueue.front().data1;
 }
 
 size_t XmlPullParser::getLineNumber() const {
diff --git a/tools/aapt2/XmlPullParser.h b/tools/aapt2/XmlPullParser.h
index f7d7a03a4352b..a0ce21dd3218f 100644
--- a/tools/aapt2/XmlPullParser.h
+++ b/tools/aapt2/XmlPullParser.h
@@ -158,7 +158,6 @@ private:
         size_t depth;
         std::u16string data1;
         std::u16string data2;
-        std::u16string comment;
         std::vector<Attribute> attributes;
     };
 
diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp
index 095552aa7f316..47fa2a63d5533 100644
--- a/tools/aapt2/flatten/TableFlattener.cpp
+++ b/tools/aapt2/flatten/TableFlattener.cpp
@@ -292,7 +292,7 @@ private:
     SymbolWriter* mSymbols;
     StringPool* mSourcePool;
 
-    template <typename T>
+    template <typename T, bool IsItem>
     T* writeEntry(FlatEntry* entry, BigBuffer* buffer) {
         static_assert(std::is_same<ResTable_entry, T>::value ||
                       std::is_same<ResTable_entry_ext, T>::value,
@@ -308,7 +308,7 @@ private:
             outEntry->flags |= ResTable_entry::FLAG_WEAK;
         }
 
-        if (!entry->value->isItem()) {
+        if (!IsItem) {
             outEntry->flags |= ResTable_entry::FLAG_COMPLEX;
         }
 
@@ -329,8 +329,8 @@ private:
     }
 
     bool flattenValue(FlatEntry* entry, BigBuffer* buffer) {
-        if (entry->value->isItem()) {
-            writeEntry<ResTable_entry>(entry, buffer);
+        if (Item* item = valueCast<Item>(entry->value)) {
+            writeEntry<ResTable_entry, true>(entry, buffer);
             if (Reference* ref = valueCast<Reference>(entry->value)) {
                 if (!ref->id) {
                     assert(ref->name && "reference must have at least a name");
@@ -339,12 +339,12 @@ private:
                 }
             }
             Res_value* outValue = buffer->nextBlock<Res_value>();
-            bool result = static_cast<Item*>(entry->value)->flatten(outValue);
+            bool result = item->flatten(outValue);
             assert(result && "flatten failed");
             outValue->size = util::hostToDevice16(sizeof(*outValue));
         } else {
             const size_t beforeEntry = buffer->size();
-            ResTable_entry_ext* outEntry = writeEntry<ResTable_entry_ext>(entry, buffer);
+            ResTable_entry_ext* outEntry = writeEntry<ResTable_entry_ext, false>(entry, buffer);
             MapFlattenVisitor visitor(mSymbols, entry, buffer);
             entry->value->accept(&visitor);
             outEntry->count = util::hostToDevice32(visitor.mEntryCount);
@@ -551,17 +551,27 @@ private:
             // configuration available. Here we reverse this to match the binary table.
             std::map<ConfigDescription, std::vector<FlatEntry>> configToEntryListMap;
             for (ResourceEntry* entry : sortedEntries) {
-                const size_t keyIndex = mKeyPool.makeRef(entry->name).getIndex();
+                const uint32_t keyIndex = (uint32_t) mKeyPool.makeRef(entry->name).getIndex();
 
                 // Group values by configuration.
                 for (auto& configValue : entry->values) {
-                   configToEntryListMap[configValue.config].push_back(FlatEntry{
-                            entry, configValue.value.get(), (uint32_t) keyIndex,
-                            (uint32_t)(mSourcePool->makeRef(util::utf8ToUtf16(
-                                    configValue.source.path)).getIndex()),
-                            (uint32_t)(configValue.source.line
-                                    ? configValue.source.line.value() : 0)
-                   });
+                    Value* value = configValue.value.get();
+
+                    const StringPool::Ref sourceRef = mSourcePool->makeRef(
+                            util::utf8ToUtf16(value->getSource().path));
+
+                    uint32_t lineNumber = 0;
+                    if (value->getSource().line) {
+                        lineNumber = value->getSource().line.value();
+                    }
+
+                    configToEntryListMap[configValue.config]
+                            .push_back(FlatEntry{
+                                    entry,
+                                    value,
+                                    keyIndex,
+                                    (uint32_t) sourceRef.getIndex(),
+                                    lineNumber });
                 }
             }
 
diff --git a/tools/aapt2/link/AutoVersioner.cpp b/tools/aapt2/link/AutoVersioner.cpp
index 0ccafc2107d01..11fcc5d6274d9 100644
--- a/tools/aapt2/link/AutoVersioner.cpp
+++ b/tools/aapt2/link/AutoVersioner.cpp
@@ -20,21 +20,18 @@
 #include "ValueVisitor.h"
 
 #include "link/Linkers.h"
+#include "util/Comparators.h"
 
 #include <algorithm>
 #include <cassert>
 
 namespace aapt {
 
-static bool cmpConfigValue(const ResourceConfigValue& lhs, const ConfigDescription& config) {
-    return lhs.config < config;
-}
-
 bool shouldGenerateVersionedResource(const ResourceEntry* entry, const ConfigDescription& config,
                                      const int sdkVersionToGenerate) {
     assert(sdkVersionToGenerate > config.sdkVersion);
     const auto endIter = entry->values.end();
-    auto iter = std::lower_bound(entry->values.begin(), endIter, config, cmpConfigValue);
+    auto iter = std::lower_bound(entry->values.begin(), endIter, config, cmp::lessThan);
 
     // The source config came from this list, so it should be here.
     assert(iter != entry->values.end());
@@ -107,21 +104,16 @@ bool AutoVersioner::consume(IAaptContext* context, ResourceTable* table) {
                             // We found attributes from a higher SDK level. Check that
                             // there is no other defined resource for the version we want to
                             // generate.
-                            if (shouldGenerateVersionedResource(entry.get(), configValue.config,
+                            if (shouldGenerateVersionedResource(entry.get(),
+                                                                configValue.config,
                                                                 minSdkStripped.value())) {
                                 // Let's create a new Style for this versioned resource.
                                 ConfigDescription newConfig(configValue.config);
                                 newConfig.sdkVersion = minSdkStripped.value();
 
-                                ResourceConfigValue newValue = {
-                                        newConfig,
-                                        configValue.source,
-                                        configValue.comment,
-                                        std::unique_ptr<Value>(configValue.value->clone(
-                                                &table->stringPool))
-                                };
-
-                                Style* newStyle = static_cast<Style*>(newValue.value.get());
+                                std::unique_ptr<Style> newStyle(style->clone(&table->stringPool));
+                                newStyle->setComment(style->getComment());
+                                newStyle->setSource(style->getSource());
 
                                 // Move the previously stripped attributes into this style.
                                 newStyle->entries.insert(newStyle->entries.end(),
@@ -130,9 +122,13 @@ bool AutoVersioner::consume(IAaptContext* context, ResourceTable* table) {
 
                                 // Insert the new Resource into the correct place.
                                 auto iter = std::lower_bound(entry->values.begin(),
-                                                             entry->values.end(), newConfig,
-                                                             cmpConfigValue);
-                                entry->values.insert(iter, std::move(newValue));
+                                                             entry->values.end(),
+                                                             newConfig,
+                                                             cmp::lessThan);
+
+                                entry->values.insert(
+                                        iter,
+                                        ResourceConfigValue{ newConfig, std::move(newStyle) });
                             }
                         }
                     }
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index b84f2e068dc27..ad701deac16f8 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -266,13 +266,14 @@ struct LinkCommand {
                 for (const auto& type : package->types) {
                     for (const auto& entry : type->entries) {
                         for (const auto& configValue : entry->values) {
-                            mContext.getDiagnostics()->error(DiagMessage(configValue.source)
-                                                             << "defined resource '"
-                                                             << ResourceNameRef(package->name,
-                                                                                type->type,
-                                                                                entry->name)
-                                                             << "' for external package '"
-                                                             << package->name << "'");
+                            mContext.getDiagnostics()->error(
+                                    DiagMessage(configValue.value->getSource())
+                                                << "defined resource '"
+                                                << ResourceNameRef(package->name,
+                                                                   type->type,
+                                                                   entry->name)
+                                                << "' for external package '"
+                                                << package->name << "'");
                             error = true;
                         }
                     }
@@ -472,10 +473,11 @@ struct LinkCommand {
 
                         Maybe<ResourceName> mangledName = mContext.getNameMangler()->mangleName(
                                 exportedSymbol.name);
-                        if (!mergedTable.addResource(
+                        std::unique_ptr<Id> id = util::make_unique<Id>();
+                        id->setSource(f.source.withLine(exportedSymbol.line));
+                        if (!mergedTable.addResourceAllowMangled(
                                 mangledName ? mangledName.value() : exportedSymbol.name,
-                                {}, {}, f.source.withLine(exportedSymbol.line),
-                                util::make_unique<Id>(), mContext.getDiagnostics())) {
+                                {}, std::move(id), mContext.getDiagnostics())) {
                             error = true;
                         }
                     }
diff --git a/tools/aapt2/link/PrivateAttributeMover_test.cpp b/tools/aapt2/link/PrivateAttributeMover_test.cpp
index a2f8d19b57cca..dbe0c92253c1e 100644
--- a/tools/aapt2/link/PrivateAttributeMover_test.cpp
+++ b/tools/aapt2/link/PrivateAttributeMover_test.cpp
@@ -30,13 +30,9 @@ TEST(PrivateAttributeMoverTest, MovePrivateAttributes) {
             .addSimple(u"@android:attr/privateA")
             .addSimple(u"@android:attr/publicB")
             .addSimple(u"@android:attr/privateB")
+            .setSymbolState(u"@android:attr/publicA", ResourceId(0x01010000), SymbolState::kPublic)
+            .setSymbolState(u"@android:attr/publicB", ResourceId(0x01010000), SymbolState::kPublic)
             .build();
-    ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:attr/publicA"),
-                                      ResourceId(0x01010000), {}, SymbolState::kPublic,
-                                      context->getDiagnostics()));
-    ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:attr/publicB"),
-                                      ResourceId(0x01010002), {}, SymbolState::kPublic,
-                                      context->getDiagnostics()));
 
     PrivateAttributeMover mover;
     ASSERT_TRUE(mover.consume(context.get(), table.get()));
diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index b4fb9961f0c54..8c924b5b0f99b 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -206,13 +206,13 @@ public:
 
                 if (!(typeMask & ResourceUtils::androidTypeToAttributeTypeMask(val.dataType))) {
                     // The actual type of this item is incompatible with the attribute.
-                    DiagMessage msg;
+                    DiagMessage msg(style->getSource());
                     buildAttributeMismatchMessage(&msg, s->attribute.get(), entry.value.get());
                     mContext->getDiagnostics()->error(msg);
                     mError = true;
                 }
             } else {
-                DiagMessage msg;
+                DiagMessage msg(style->getSource());
                 msg << "style attribute '";
                 if (entry.key.name) {
                     msg << entry.key.name.value().package << ":" << entry.key.name.value().entry;
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index db525467933f7..636c2ba9e9fd6 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -19,6 +19,7 @@
 #include "ValueVisitor.h"
 
 #include "link/TableMerger.h"
+#include "util/Comparators.h"
 #include "util/Util.h"
 
 #include <cassert>
@@ -120,27 +121,24 @@ bool TableMerger::doMerge(const Source& src, ResourceTable* srcTable,
             }
 
             for (ResourceConfigValue& srcValue : srcEntry->values) {
-                auto cmp = [](const ResourceConfigValue& a,
-                              const ConfigDescription& b) -> bool {
-                    return a.config < b;
-                };
-
                 auto iter = std::lower_bound(dstEntry->values.begin(), dstEntry->values.end(),
-                                             srcValue.config, cmp);
+                                             srcValue.config, cmp::lessThan);
 
                 if (iter != dstEntry->values.end() && iter->config == srcValue.config) {
                     const int collisionResult = ResourceTable::resolveValueCollision(
                             iter->value.get(), srcValue.value.get());
                     if (collisionResult == 0) {
                         // Error!
-                        ResourceNameRef resourceName =
-                                { srcPackage->name, srcType->type, srcEntry->name };
-                        mContext->getDiagnostics()->error(DiagMessage(srcValue.source)
+                        ResourceNameRef resourceName(srcPackage->name,
+                                                     srcType->type,
+                                                     srcEntry->name);
+
+                        mContext->getDiagnostics()->error(DiagMessage(srcValue.value->getSource())
                                                           << "resource '" << resourceName
                                                           << "' has a conflicting value for "
                                                           << "configuration ("
                                                           << srcValue.config << ")");
-                        mContext->getDiagnostics()->note(DiagMessage(iter->source)
+                        mContext->getDiagnostics()->note(DiagMessage(iter->value->getSource())
                                                          << "originally defined here");
                         error = true;
                         continue;
@@ -150,16 +148,12 @@ bool TableMerger::doMerge(const Source& src, ResourceTable* srcTable,
                     }
 
                 } else {
-                    // Insert a new value.
-                    iter = dstEntry->values.insert(iter,
-                                                   ResourceConfigValue{ srcValue.config });
+                    // Insert a place holder value. We will fill it in below.
+                    iter = dstEntry->values.insert(iter, ResourceConfigValue{ srcValue.config });
                 }
 
-                iter->source = std::move(srcValue.source);
-                iter->comment = std::move(srcValue.comment);
                 if (manglePackage) {
-                    iter->value = cloneAndMangle(srcTable, srcPackage->name,
-                                                 srcValue.value.get());
+                    iter->value = cloneAndMangle(srcTable, srcPackage->name, srcValue.value.get());
                 } else {
                     iter->value = clone(srcValue.value.get());
                 }
@@ -179,7 +173,11 @@ std::unique_ptr<Value> TableMerger::cloneAndMangle(ResourceTable* table,
             std::u16string mangledEntry = NameMangler::mangleEntry(package, entry.toString());
             std::u16string newPath = prefix.toString() + mangledEntry + suffix.toString();
             mFilesToMerge.push(FileToMerge{ table, *f->path, newPath });
-            return util::make_unique<FileReference>(mMasterTable->stringPool.makeRef(newPath));
+            std::unique_ptr<FileReference> fileRef = util::make_unique<FileReference>(
+                    mMasterTable->stringPool.makeRef(newPath));
+            fileRef->setComment(f->getComment());
+            fileRef->setSource(f->getSource());
+            return std::move(fileRef);
         }
     }
     return clone(value);
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index c96b080f586b8..7309396547e6a 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -16,9 +16,11 @@
 
 #include "ConfigDescription.h"
 #include "Resource.h"
-#include "util/Util.h"
+#include "ValueVisitor.h"
 
 #include "process/SymbolTable.h"
+#include "util/Comparators.h"
+#include "util/Util.h"
 
 #include <androidfw/AssetManager.h>
 #include <androidfw/ResourceTypes.h>
@@ -34,7 +36,7 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n
     if (!result) {
         if (name.type == ResourceType::kAttr) {
             // Recurse and try looking up a private attribute.
-            return findByName(ResourceName{ name.package, ResourceType::kAttrPrivate, name.entry });
+            return findByName(ResourceName(name.package, ResourceType::kAttrPrivate, name.entry));
         }
         return {};
     }
@@ -48,28 +50,26 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n
     }
 
     std::shared_ptr<Symbol> symbol = std::make_shared<Symbol>();
-    symbol->id = ResourceId{
-            sr.package->id.value(), sr.type->id.value(), sr.entry->id.value() };
+    symbol->id = ResourceId(sr.package->id.value(), sr.type->id.value(), sr.entry->id.value());
 
     if (name.type == ResourceType::kAttr || name.type == ResourceType::kAttrPrivate) {
-        auto lt = [](ResourceConfigValue& lhs, const ConfigDescription& rhs) -> bool {
-            return lhs.config < rhs;
-        };
-
         const ConfigDescription kDefaultConfig;
         auto iter = std::lower_bound(sr.entry->values.begin(), sr.entry->values.end(),
-                                     kDefaultConfig, lt);
+                                     kDefaultConfig, cmp::lessThan);
 
         if (iter != sr.entry->values.end() && iter->config == kDefaultConfig) {
             // This resource has an Attribute.
-            symbol->attribute = util::make_unique<Attribute>(
-                    *static_cast<Attribute*>(iter->value.get()));
+            if (Attribute* attr = valueCast<Attribute>(iter->value.get())) {
+                symbol->attribute = std::unique_ptr<Attribute>(attr->clone(nullptr));
+            } else {
+                return {};
+            }
         }
     }
 
     if (name.type == ResourceType::kAttrPrivate) {
         // Masquerade this entry as kAttr.
-        mCache.put(ResourceName{ name.package, ResourceType::kAttr, name.entry }, symbol);
+        mCache.put(ResourceName(name.package, ResourceType::kAttr, name.entry), symbol);
     } else {
         mCache.put(name, symbol);
     }
diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h
index 0383c443d4c10..1b510e756855a 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -43,7 +43,7 @@ public:
         return *this;
     }
 
-    ResourceTableBuilder& addSimple(const StringPiece16& name, ResourceId id = {}) {
+    ResourceTableBuilder& addSimple(const StringPiece16& name, const ResourceId id = {}) {
         return addValue(name, id, util::make_unique<Id>());
     }
 
@@ -51,7 +51,7 @@ public:
         return addReference(name, {}, ref);
     }
 
-    ResourceTableBuilder& addReference(const StringPiece16& name, ResourceId id,
+    ResourceTableBuilder& addReference(const StringPiece16& name, const ResourceId id,
                                        const StringPiece16& ref) {
         return addValue(name, id, util::make_unique<Reference>(parseNameOrDie(ref)));
     }
@@ -60,7 +60,7 @@ public:
         return addString(name, {}, str);
     }
 
-    ResourceTableBuilder& addString(const StringPiece16& name, ResourceId id,
+    ResourceTableBuilder& addString(const StringPiece16& name, const ResourceId id,
                                     const StringPiece16& str) {
         return addValue(name, id, util::make_unique<String>(mTable->stringPool.makeRef(str)));
     }
@@ -69,31 +69,43 @@ public:
         return addFileReference(name, {}, path);
     }
 
-    ResourceTableBuilder& addFileReference(const StringPiece16& name, ResourceId id,
+    ResourceTableBuilder& addFileReference(const StringPiece16& name, const ResourceId id,
                                            const StringPiece16& path) {
         return addValue(name, id,
                         util::make_unique<FileReference>(mTable->stringPool.makeRef(path)));
     }
 
 
-    ResourceTableBuilder& addValue(const StringPiece16& name, std::unique_ptr<Value> value) {
+    ResourceTableBuilder& addValue(const StringPiece16& name,
+                                   std::unique_ptr<Value> value) {
         return addValue(name, {}, std::move(value));
     }
 
-    ResourceTableBuilder& addValue(const StringPiece16& name, ResourceId id,
+    ResourceTableBuilder& addValue(const StringPiece16& name, const ResourceId id,
                                        std::unique_ptr<Value> value) {
         return addValue(name, id, {}, std::move(value));
     }
 
-    ResourceTableBuilder& addValue(const StringPiece16& name, ResourceId id,
-                                   const ConfigDescription& config, std::unique_ptr<Value> value) {
+    ResourceTableBuilder& addValue(const StringPiece16& name, const ResourceId id,
+                                   const ConfigDescription& config,
+                                   std::unique_ptr<Value> value) {
         ResourceName resName = parseNameOrDie(name);
-        bool result = mTable->addResourceAllowMangled(resName, id, config, {}, std::move(value),
+        bool result = mTable->addResourceAllowMangled(resName, id, config, std::move(value),
                                                       &mDiagnostics);
         assert(result);
         return *this;
     }
 
+    ResourceTableBuilder& setSymbolState(const StringPiece16& name, ResourceId id,
+                                         SymbolState state) {
+        ResourceName resName = parseNameOrDie(name);
+        Symbol symbol;
+        symbol.state = state;
+        bool result = mTable->setSymbolStateAllowMangled(resName, id, symbol, &mDiagnostics);
+        assert(result);
+        return *this;
+    }
+
     std::unique_ptr<ResourceTable> build() {
         return std::move(mTable);
     }
@@ -106,6 +118,32 @@ inline std::unique_ptr<Reference> buildReference(const StringPiece16& ref,
     return reference;
 }
 
+template <typename T>
+class ValueBuilder {
+private:
+    std::unique_ptr<Value> mValue;
+
+public:
+    template <typename... Args>
+    ValueBuilder(Args&&... args) : mValue(new T{ std::forward<Args>(args)... }) {
+    }
+
+    template <typename... Args>
+    ValueBuilder& setSource(Args&&... args) {
+        mValue->setSource(Source{ std::forward<Args>(args)... });
+        return *this;
+    }
+
+    ValueBuilder& setComment(const StringPiece16& str) {
+        mValue->setComment(str);
+        return *this;
+    }
+
+    std::unique_ptr<Value> build() {
+        return std::move(mValue);
+    }
+};
+
 class AttributeBuilder {
 private:
     std::unique_ptr<Attribute> mAttr;
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index c7a715e7d2e27..314c1e84f44fb 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -422,26 +422,24 @@ bool BinaryResourceParser::parsePublic(const ResourceTablePackage* package,
         const ResourceName name(package->name, *parsedType,
                                 util::getString(mKeyPool, entry->key.index).toString());
 
-        Source source;
+        Symbol symbol;
         if (mSourcePool.getError() == NO_ERROR) {
-            source.path = util::utf16ToUtf8(util::getString(
+            symbol.source.path = util::utf16ToUtf8(util::getString(
                     mSourcePool, util::deviceToHost32(entry->source.index)));
-            source.line = util::deviceToHost32(entry->sourceLine);
+            symbol.source.line = util::deviceToHost32(entry->sourceLine);
         }
 
-        SymbolState state = SymbolState::kUndefined;
         switch (util::deviceToHost16(entry->state)) {
         case Public_entry::kPrivate:
-            state = SymbolState::kPrivate;
+            symbol.state = SymbolState::kPrivate;
             break;
 
         case Public_entry::kPublic:
-            state = SymbolState::kPublic;
+            symbol.state = SymbolState::kPublic;
             break;
         }
 
-        if (!mTable->setSymbolStateAllowMangled(name, resId, source, state,
-                                                mContext->getDiagnostics())) {
+        if (!mTable->setSymbolStateAllowMangled(name, resId, symbol, mContext->getDiagnostics())) {
             return false;
         }
 
@@ -570,14 +568,17 @@ bool BinaryResourceParser::parseType(const ResourceTablePackage* package,
             source.line = util::deviceToHost32(sourceBlock->line);
         }
 
-        if (!mTable->addResourceAllowMangled(name, config, source, std::move(resourceValue),
+        resourceValue->setSource(source);
+        if (!mTable->addResourceAllowMangled(name, config, std::move(resourceValue),
                                              mContext->getDiagnostics())) {
             return false;
         }
 
         if ((entry->flags & ResTable_entry::FLAG_PUBLIC) != 0) {
-            if (!mTable->setSymbolStateAllowMangled(name, resId, mSource.withLine(0),
-                                                    SymbolState::kPublic,
+            Symbol symbol;
+            symbol.state = SymbolState::kPublic;
+            symbol.source = mSource.withLine(0);
+            if (!mTable->setSymbolStateAllowMangled(name, resId, symbol,
                                                     mContext->getDiagnostics())) {
                 return false;
             }
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.h b/tools/aapt2/unflatten/BinaryResourceParser.h
index 4dbef5dcb8154..02c4081cc0e26 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.h
+++ b/tools/aapt2/unflatten/BinaryResourceParser.h
@@ -66,26 +66,30 @@ private:
     bool parseTypeSpec(const android::ResChunk_header* chunk);
     bool parseType(const ResourceTablePackage* package, const android::ResChunk_header* chunk);
 
-    std::unique_ptr<Item> parseValue(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::Res_value* value, uint16_t flags);
+    std::unique_ptr<Item> parseValue(const ResourceNameRef& name, const ConfigDescription& config,
+                                     const android::Res_value* value, uint16_t flags);
 
     std::unique_ptr<Value> parseMapEntry(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::ResTable_map_entry* map);
+                                         const ConfigDescription& config,
+                                         const android::ResTable_map_entry* map);
 
-    std::unique_ptr<Style> parseStyle(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::ResTable_map_entry* map);
+    std::unique_ptr<Style> parseStyle(const ResourceNameRef& name, const ConfigDescription& config,
+                                      const android::ResTable_map_entry* map);
 
     std::unique_ptr<Attribute> parseAttr(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::ResTable_map_entry* map);
+                                         const ConfigDescription& config,
+                                         const android::ResTable_map_entry* map);
 
-    std::unique_ptr<Array> parseArray(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::ResTable_map_entry* map);
+    std::unique_ptr<Array> parseArray(const ResourceNameRef& name, const ConfigDescription& config,
+                                      const android::ResTable_map_entry* map);
 
     std::unique_ptr<Plural> parsePlural(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::ResTable_map_entry* map);
+                                        const ConfigDescription& config,
+                                        const android::ResTable_map_entry* map);
 
     std::unique_ptr<Styleable> parseStyleable(const ResourceNameRef& name,
-            const ConfigDescription& config, const android::ResTable_map_entry* map);
+                                              const ConfigDescription& config,
+                                              const android::ResTable_map_entry* map);
 
     IAaptContext* mContext;
     ResourceTable* mTable;
diff --git a/tools/aapt2/util/Comparators.h b/tools/aapt2/util/Comparators.h
new file mode 100644
index 0000000000000..652018e342c7a
--- /dev/null
+++ b/tools/aapt2/util/Comparators.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef AAPT_UTIL_COMPARATORS_H
+#define AAPT_UTIL_COMPARATORS_H
+
+namespace aapt {
+namespace cmp {
+
+inline bool lessThan(const ResourceConfigValue& a, const ConfigDescription& b) {
+    return a.config < b;
+}
+
+} // namespace cmp
+} // namespace aapt
+
+#endif /* AAPT_UTIL_COMPARATORS_H */