[YAML] Use correct source location for unknown key errors.

[njames93]

Summary

Currently unknown keys when inputting mapping traits have the location set to the Value.
Example:

YAML:1:14: error: unknown key 'UnknownKey'
{UnknownKey: SomeValue}
             ^~~~~~~~~

This is unhelpful for a user as it draws them to fix the wrong item.

[silvas]

Would be nice to find a way to test this (Maybe a FileCheck test? not sure how to set that up though; don't worry if it is too much work.)

[njames93]

In D93037#2446301, @silvas wrote:

Would be nice to find a way to test this (Maybe a FileCheck test? not sure how to set that up though; don't worry if it is too much work.)

Bit of a pain to set that up explicitly. Thankfully, there are 8 test failures(per platform) introduced by this patch that demonstrate the old unexpected behaviour.
Fixing those should be semi-decent basic coverage.

[njames93]

Fix failing tests introduced with this patch.

llvm/include/llvm/Support/YAMLParser.h

public:
 
   bool validate() {
     skip();
     return !failed();
   }
 
   void printError(Node *N, const Twine &Msg,
                   SourceMgr::DiagKind Kind = SourceMgr::DK_Error);
+  void printError(const SMRange &Range, const Twine &Msg,
+                  SourceMgr::DiagKind Kind = SourceMgr::DK_Error);
 
 private:
   friend class Document;
 
   std::unique_ptr<Scanner> scanner;
   std::unique_ptr<Document> CurrentDoc;
 };
 

llvm/include/llvm/Support/YAMLTraits.h

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cctype>
 #include <cstddef>
 #include <cstdint>
public:
     MapHNode(Node *n) : HNode(n) { }
 
     static bool classof(const HNode *n) {
       return MappingNode::classof(n->_node);
     }
 
     static bool classof(const MapHNode *) { return true; }
 
-    using NameToNode = StringMap<std::unique_ptr<HNode>>;
+    using NameToNodeAndLoc =
+        StringMap<std::pair<std::unique_ptr<HNode>, SMRange>>;
 
-    NameToNode Mapping;
+    NameToNodeAndLoc Mapping;
     SmallVector<std::string, 6> ValidKeys;
   };
 
   class SequenceHNode : public HNode {
     void anchor() override;
 
   public:
     SequenceHNode(Node *n) : HNode(n) { }
 
     static bool classof(const HNode *n) {
       return SequenceNode::classof(n->_node);
     }
 
     static bool classof(const SequenceHNode *) { return true; }
 
     std::vector<std::unique_ptr<HNode>> Entries;
   };
 
   std::unique_ptr<Input::HNode> createHNodes(Node *node);
   void setError(HNode *hnode, const Twine &message);
   void setError(Node *node, const Twine &message);
+  void setError(const SMRange &Range, const Twine &message);
 
   void reportWarning(HNode *hnode, const Twine &message);
   void reportWarning(Node *hnode, const Twine &message);
+  void reportWarning(const SMRange &Range, const Twine &message);
 
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
   bool setCurrentDocument();
   bool nextDocument();
 
   /// Returns the current node that's being parsed by the YAML Parser.

llvm/lib/Support/YAMLParser.cpp

Stream::Stream(MemoryBufferRef InputBuffer, SourceMgr &SM, bool ShowColors,
                std::error_code *EC)
     : scanner(new Scanner(InputBuffer, SM, ShowColors, EC)), CurrentDoc() {}
 
 Stream::~Stream() = default;
 
 bool Stream::failed() { return scanner->failed(); }
 
 void Stream::printError(Node *N, const Twine &Msg, SourceMgr::DiagKind Kind) {
-  SMRange Range = N ? N->getSourceRange() : SMRange();
+  printError(N ? N->getSourceRange() : SMRange(), Msg, Kind);
+}
+
+void Stream::printError(const SMRange &Range, const Twine &Msg,
+                        SourceMgr::DiagKind Kind) {
   scanner->printError(Range.Start, Kind, Msg, Range);
 }
 
 document_iterator Stream::begin() {
   if (CurrentDoc)
     report_fatal_error("Can only iterate over the stream once");
 
   // Skip Stream-Start.

llvm/lib/Support/YAMLTraits.cpp

bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault,
   if (!MN) {
     if (Required || !isa<EmptyHNode>(CurrentNode))
       setError(CurrentNode, "not a mapping");
     else
       UseDefault = true;
     return false;
   }
   MN->ValidKeys.push_back(Key);
-  HNode *Value = MN->Mapping[Key].get();
+  HNode *Value = MN->Mapping[Key].first.get();
   if (!Value) {
     if (Required)
       setError(CurrentNode, Twine("missing required key '") + Key + "'");
     else
       UseDefault = true;
     return false;
   }
   SaveInfo = CurrentNode;
void Input::endMapping() {
   if (EC)
     return;
   // CurrentNode can be null if the document is empty.
   MapHNode *MN = dyn_cast_or_null<MapHNode>(CurrentNode);
   if (!MN)
     return;
   for (const auto &NN : MN->Mapping) {
     if (!is_contained(MN->ValidKeys, NN.first())) {
-      HNode *ReportNode = NN.second.get();
+      const SMRange &ReportLoc = NN.second.second;
       if (!AllowUnknownKeys) {
-        setError(ReportNode, Twine("unknown key '") + NN.first() + "'");
+        setError(ReportLoc, Twine("unknown key '") + NN.first() + "'");
         break;
       } else
-        reportWarning(ReportNode, Twine("unknown key '") + NN.first() + "'");
+        reportWarning(ReportLoc, Twine("unknown key '") + NN.first() + "'");
     }
   }
 }
 
 void Input::beginFlowMapping() { beginMapping(); }
 
 void Input::endFlowMapping() { endMapping(); }
 
NodeKind Input::getNodeKind() {
   llvm_unreachable("Unsupported node kind");
 }
 
 void Input::setError(Node *node, const Twine &message) {
   Strm->printError(node, message);
   EC = make_error_code(errc::invalid_argument);
 }
 
+void Input::setError(const SMRange &range, const Twine &message) {
+  Strm->printError(range, message);
+  EC = make_error_code(errc::invalid_argument);
+}
+
 void Input::reportWarning(HNode *hnode, const Twine &message) {
   assert(hnode && "HNode must not be NULL");
   Strm->printError(hnode->_node, message, SourceMgr::DK_Warning);
 }
 
+void Input::reportWarning(Node *node, const Twine &message) {
+  Strm->printError(node, message, SourceMgr::DK_Warning);
+}
+
+void Input::reportWarning(const SMRange &range, const Twine &message) {
+  Strm->printError(range, message, SourceMgr::DK_Warning);
+}
+
 std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
   SmallString<128> StringStorage;
   if (ScalarNode *SN = dyn_cast<ScalarNode>(N)) {
     StringRef KeyStr = SN->getValue(StringStorage);
     if (!StringStorage.empty()) {
       // Copy string to permanent storage
       KeyStr = StringStorage.str().copy(StringAllocator);
     }
for (KeyValueNode &KVN : *Map) {
       StringRef KeyStr = Key->getValue(StringStorage);
       if (!StringStorage.empty()) {
         // Copy string to permanent storage
         KeyStr = StringStorage.str().copy(StringAllocator);
       }
       auto ValueHNode = createHNodes(Value);
       if (EC)
         break;
-      mapHNode->Mapping[KeyStr] = std::move(ValueHNode);
+      mapHNode->Mapping[KeyStr] =
+          std::make_pair(std::move(ValueHNode), KeyNode->getSourceRange());
     }
     return std::move(mapHNode);
   } else if (isa<NullNode>(N)) {
     return std::make_unique<EmptyHNode>(N);
   } else {
     setError(N, "unknown node kind");
     return nullptr;
   }

llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-aliased.mir

--- |
   attributes #0 = { "frame-pointer"="none" }
 
 ...
 ---
 name:            test
 frameInfo:
   maxAlignment:    4
 fixedStack:
-  # CHECK: [[@LINE+1]]:63: unknown key 'isAliased'
+  # CHECK: [[@LINE+1]]:52: unknown key 'isAliased'
   - { id: 0, type: spill-slot, offset: 0, size: 4, isAliased: true }
 stack:
   - { id: 0, offset: -12, size: 4, alignment: 4 }
 body: |
   bb.0.entry:
     MOV32mr %rsp, 1, _, -4, _, %edi
     %eax = COPY %edi
     RETQ %eax
 ...

llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-immutable.mir

--- |
   attributes #0 = { "frame-pointer"="none" }
 
 ...
 ---
 name:            test
 frameInfo:
   maxAlignment:    4
 fixedStack:
-  # CHECK: [[@LINE+1]]:65: unknown key 'isImmutable'
+  # CHECK: [[@LINE+1]]:52: unknown key 'isImmutable'
   - { id: 0, type: spill-slot, offset: 0, size: 4, isImmutable: true }
 stack:
   - { id: 0, offset: -12, size: 4, alignment: 4 }
 body: |
   bb.0.entry:
     MOV32mr %rsp, 1, _, -4, _, %edi
     %eax = COPY %edi
     RETQ %eax
 ...

llvm/test/CodeGen/MIR/X86/variable-sized-stack-object-size-error.mir

 frameInfo:
   stackSize:       24
   offsetAdjustment: -16
   maxAlignment:    8
   adjustsStack:    true
 stack:
   - { id: 0, offset: -20, size: 4, alignment: 4 }
   - { id: 1, offset: -32, size: 8, alignment: 8 }
-  # CHECK: [[@LINE+1]]:55: unknown key 'size'
+  # CHECK: [[@LINE+1]]:49: unknown key 'size'
   - { id: 2, type: variable-sized, offset: -32, size: 42, alignment: 1 }
 body: |
   bb.0.entry:
     MOV32mr %rsp, 1, _, -4, _, %edi
     MOV64mi32 %rsp, 1, _, -16, _, 2
     %eax = MOV32rm %rsp, 1, _, -4, _
     RETQ %eax
 ...

llvm/test/Object/nm-tapi-invalids.test

 RUN: not llvm-nm %p/Inputs/tapi-invalid-v1.tbd 2>&1\
 RUN:          | FileCheck %s -check-prefix V1 
 
 RUN: not llvm-nm %p/Inputs/tapi-invalid-v2.tbd 2>&1\
 RUN:          | FileCheck %s -check-prefix V2
 
 RUN: not llvm-nm %p/Inputs/tapi-invalid-v3.tbd 2>&1\
 RUN:          | FileCheck %s -check-prefix V3
 
 # Typo Check
 V1: tapi-invalid-v1.tbd malformed file
-V1-NEXT: tapi-invalid-v1.tbd:12:2: error: unknown key 'expors'
+V1-NEXT: tapi-invalid-v1.tbd:11:1: error: unknown key 'expors'
 
 # Missing required key
 V2: tapi-invalid-v2.tbd malformed file
 V2-NEXT: tapi-invalid-v2.tbd:2:1: error: missing required key 'archs'
 
 # v2 key in v3 specified file
 V3: tapi-invalid-v3.tbd malformed file
-V3-NEXT: tapi-invalid-v3.tbd:19:16: error: unknown key 'swift-version'
+V3-NEXT: tapi-invalid-v3.tbd:19:1: error: unknown key 'swift-version'

llvm/unittests/TextAPI/TextStubV1Tests.cpp

static const char TBDv1FileMalformed2[] = "---\n"
                                             "foobar: \"Unsupported key\"\n"
                                             "...\n";
 
   Expected<TBDFile> Result =
       TextAPIReader::get(MemoryBufferRef(TBDv1FileMalformed2, "Test.tbd"));
   EXPECT_FALSE(!!Result);
   std::string ErrorMessage = toString(Result.takeError());
   ASSERT_EQ(
-      "malformed file\nTest.tbd:5:9: error: unknown key 'foobar'\nfoobar: "
-      "\"Unsupported key\"\n        ^~~~~~~~~~~~~~~~~\n",
+      "malformed file\nTest.tbd:5:1: error: unknown key 'foobar'\nfoobar: "
+      "\"Unsupported key\"\n^~~~~~\n",
       ErrorMessage);
 }
 
 } // end namespace TBDv1.

llvm/unittests/TextAPI/TextStubV2Tests.cpp

static const char TBDv2FileMalformed2[] = "--- !tapi-tbd-v2\n"
                                             "foobar: \"Unsupported key\"\n"
                                             "...\n";
 
   Expected<TBDFile> Result =
       TextAPIReader::get(MemoryBufferRef(TBDv2FileMalformed2, "Test.tbd"));
   EXPECT_FALSE(!!Result);
   std::string ErrorMessage = toString(Result.takeError());
   ASSERT_EQ(
-      "malformed file\nTest.tbd:5:9: error: unknown key 'foobar'\nfoobar: "
-      "\"Unsupported key\"\n        ^~~~~~~~~~~~~~~~~\n",
+      "malformed file\nTest.tbd:5:1: error: unknown key 'foobar'\nfoobar: "
+      "\"Unsupported key\"\n^~~~~~\n",
       ErrorMessage);
 }
 
 } // namespace TBDv2

llvm/unittests/TextAPI/TextStubV3Tests.cpp

static const char TBDv3FileMalformed2[] = "--- !tapi-tbd-v3\n"
                                             "foobar: \"Unsupported key\"\n"
                                             "...\n";
 
   Expected<TBDFile> Result =
       TextAPIReader::get(MemoryBufferRef(TBDv3FileMalformed2, "Test.tbd"));
   EXPECT_FALSE(!!Result);
   std::string ErrorMessage = toString(Result.takeError());
   ASSERT_EQ(
-      "malformed file\nTest.tbd:5:9: error: unknown key 'foobar'\nfoobar: "
-      "\"Unsupported key\"\n        ^~~~~~~~~~~~~~~~~\n",
+      "malformed file\nTest.tbd:5:1: error: unknown key 'foobar'\nfoobar: "
+      "\"Unsupported key\"\n^~~~~~\n",
       ErrorMessage);
 }
 
 } // namespace TBDv3

llvm/unittests/TextAPI/TextStubV4Tests.cpp

static const char TBDv4MalformedFile2[] = "--- !tapi-tbd\n"
                                             "install-name: Test.dylib\n"
                                             "foobar: \"unsupported key\"\n";
 
   Expected<TBDFile> Result =
       TextAPIReader::get(MemoryBufferRef(TBDv4MalformedFile2, "Test.tbd"));
   EXPECT_FALSE(!!Result);
   std::string ErrorMessage = toString(Result.takeError());
   ASSERT_EQ(
-      "malformed file\nTest.tbd:5:9: error: unknown key 'foobar'\nfoobar: "
-      "\"unsupported key\"\n        ^~~~~~~~~~~~~~~~~\n",
+      "malformed file\nTest.tbd:5:1: error: unknown key 'foobar'\nfoobar: "
+      "\"unsupported key\"\n^~~~~~\n",
       ErrorMessage);
 }
 
 TEST(TBDv4, MalformedFile3) {
   static const char TBDv4MalformedSwift[] = "--- !tapi-tbd\n"
                                             "tbd-version: 4\n"
                                             "targets: [  x86_64-macos ]\n"
                                             "install-name: Test.dylib\n"