[YAML IO] Check that mapping doesn't contain duplicating keys

[asi-sc]

Summary

According to YAML specification keys must be unique for a mapping node:
"The content of a mapping node is an unordered set of key/value node pairs, with
the restriction that each of the keys is unique".

[asi-sc]

Clang-format .cpp tests

[asi-sc]

Fix BOLT AArch64/plt-gnu-ld.test

[asi-sc]

Rebase

[asi-sc]

Gentle ping

[njames93]

I think this patch needs splitting up with the updates to the tests put in their own NFC patch.

[asi-sc]

Move test changes to a separate review D141848

[asi-sc]

Remove tests in yaml2obj, add unittest for YAMLIO

[asi-sc]

Ping

[jhenderson]

llvm/unittests/Support/YAMLIOTest.cpp

108–110

This comment doesn't really add anything that the test name can't capture.

113–114

There's no real need to explain this here, since the comment is by the actual code logic itself.

115–117

I see that this is the pattern used above, but is there any way of testing that the error message is correct? If not, we may well want the yaml2obj test (optionally in addition to the unit test).

[asi-sc]

Address review comments

[asi-sc]

llvm/unittests/Support/YAMLIOTest.cpp

108–110

I agree. It's just the structure of this test, but such comments don't give any additional information. So, I've just removed it.

115–117

Fixed.

[jhenderson]

LGTM.

llvm/lib/Support/YAMLTraits.cpp

for (KeyValueNode &KVN : *Map) {
         break;
       }
       StringStorage.clear();
       StringRef KeyStr = Key->getValue(StringStorage);
       if (!StringStorage.empty()) {
         // Copy string to permanent storage
         KeyStr = StringStorage.str().copy(StringAllocator);
       }
+      if (mapHNode->Mapping.count(KeyStr))
+        // From YAML spec: "The content of a mapping node is an unordered set of
+        // key/value node pairs, with the restriction that each of the keys is
+        // unique."
+        setError(KeyNode, Twine("duplicated mapping key '") + KeyStr + "'");
       auto ValueHNode = createHNodes(Value);
       if (EC)
         break;
       mapHNode->Mapping[KeyStr] =
           std::make_pair(std::move(ValueHNode), KeyNode->getSourceRange());
     }
     return std::move(mapHNode);
   } else if (isa<NullNode>(N)) {

llvm/unittests/Support/YAMLIOTest.cpp

 
 TEST(YAMLIO, TestMalformedMapRead) {
   FooBar doc;
   Input yin("{foo: 3; bar: 5}", nullptr, suppressErrorMessages);
   yin >> doc;
   EXPECT_TRUE(!!yin.error());
 }
 
+TEST(YAMLIO, TestMapDuplicatedKeysRead) {
+  auto testDiagnostic = [](const llvm::SMDiagnostic &Error, void *) {
+    EXPECT_EQ(Error.getMessage(), "duplicated mapping key 'foo'");
+  };
+  FooBar doc;
+  Input yin("{foo: 3, bar: 5, foo: 4}", nullptr, testDiagnostic);
+  yin >> doc;
+  EXPECT_TRUE(!!yin.error());
+}
+
 //
 // Test the reading of a yaml sequence of mappings
 //
 TEST(YAMLIO, TestSequenceMapRead) {
   FooBarSequence seq;
   Input yin("---\n - foo:  3\n   bar:  5\n - foo:  7\n   bar:  9\n...\n");
   yin >> seq;