From d380c38e3470c5b02a3002a7ea8d836f44086b31 Mon Sep 17 00:00:00 2001 From: Nathan James Date: Fri, 11 Dec 2020 16:34:06 +0000 Subject: [PATCH] [YAML] Use correct source location for unknown key errors. 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. Reviewed By: silvas Differential Revision: https://reviews.llvm.org/D93037 --- llvm/include/llvm/Support/YAMLParser.h | 2 ++ llvm/include/llvm/Support/YAMLTraits.h | 8 ++++++-- llvm/lib/Support/YAMLParser.cpp | 6 +++++- llvm/lib/Support/YAMLTraits.cpp | 24 +++++++++++++++++----- .../X86/spill-slot-fixed-stack-object-aliased.mir | 2 +- .../spill-slot-fixed-stack-object-immutable.mir | 2 +- .../X86/variable-sized-stack-object-size-error.mir | 2 +- llvm/test/Object/nm-tapi-invalids.test | 4 ++-- llvm/unittests/TextAPI/TextStubV1Tests.cpp | 4 ++-- llvm/unittests/TextAPI/TextStubV2Tests.cpp | 4 ++-- llvm/unittests/TextAPI/TextStubV3Tests.cpp | 4 ++-- llvm/unittests/TextAPI/TextStubV4Tests.cpp | 4 ++-- 12 files changed, 45 insertions(+), 21 deletions(-) diff --git a/llvm/include/llvm/Support/YAMLParser.h b/llvm/include/llvm/Support/YAMLParser.h index 6e98de0..a449d35 100644 --- a/llvm/include/llvm/Support/YAMLParser.h +++ b/llvm/include/llvm/Support/YAMLParser.h @@ -102,6 +102,8 @@ public: 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; diff --git a/llvm/include/llvm/Support/YAMLTraits.h b/llvm/include/llvm/Support/YAMLTraits.h index 40a9c0b..586cd64 100644 --- a/llvm/include/llvm/Support/YAMLTraits.h +++ b/llvm/include/llvm/Support/YAMLTraits.h @@ -18,6 +18,7 @@ #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" @@ -1471,9 +1472,10 @@ private: static bool classof(const MapHNode *) { return true; } - using NameToNode = StringMap>; + using NameToNodeAndLoc = + StringMap, SMRange>>; - NameToNode Mapping; + NameToNodeAndLoc Mapping; SmallVector ValidKeys; }; @@ -1495,9 +1497,11 @@ private: std::unique_ptr 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 diff --git a/llvm/lib/Support/YAMLParser.cpp b/llvm/lib/Support/YAMLParser.cpp index 755f93c..3111c71 100644 --- a/llvm/lib/Support/YAMLParser.cpp +++ b/llvm/lib/Support/YAMLParser.cpp @@ -1774,7 +1774,11 @@ 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); } diff --git a/llvm/lib/Support/YAMLTraits.cpp b/llvm/lib/Support/YAMLTraits.cpp index 1b5c79f..5b24e08 100644 --- a/llvm/lib/Support/YAMLTraits.cpp +++ b/llvm/lib/Support/YAMLTraits.cpp @@ -175,7 +175,7 @@ bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault, 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 + "'"); @@ -201,12 +201,12 @@ void Input::endMapping() { 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() + "'"); } } } @@ -378,11 +378,24 @@ void Input::setError(Node *node, const Twine &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::createHNodes(Node *N) { SmallString<128> StringStorage; if (ScalarNode *SN = dyn_cast(N)) { @@ -426,7 +439,8 @@ std::unique_ptr Input::createHNodes(Node *N) { 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(N)) { diff --git a/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-aliased.mir b/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-aliased.mir index d3adf24..13c2523 100644 --- a/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-aliased.mir +++ b/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-aliased.mir @@ -18,7 +18,7 @@ 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 } diff --git a/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-immutable.mir b/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-immutable.mir index ff87479..dba4a0b 100644 --- a/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-immutable.mir +++ b/llvm/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-immutable.mir @@ -18,7 +18,7 @@ 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 } diff --git a/llvm/test/CodeGen/MIR/X86/variable-sized-stack-object-size-error.mir b/llvm/test/CodeGen/MIR/X86/variable-sized-stack-object-size-error.mir index 2633ea59..ce3e846 100644 --- a/llvm/test/CodeGen/MIR/X86/variable-sized-stack-object-size-error.mir +++ b/llvm/test/CodeGen/MIR/X86/variable-sized-stack-object-size-error.mir @@ -23,7 +23,7 @@ frameInfo: 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: diff --git a/llvm/test/Object/nm-tapi-invalids.test b/llvm/test/Object/nm-tapi-invalids.test index 95ed521..11ded10 100644 --- a/llvm/test/Object/nm-tapi-invalids.test +++ b/llvm/test/Object/nm-tapi-invalids.test @@ -9,7 +9,7 @@ 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 @@ -17,4 +17,4 @@ 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' diff --git a/llvm/unittests/TextAPI/TextStubV1Tests.cpp b/llvm/unittests/TextAPI/TextStubV1Tests.cpp index ea49ac4..64b9769 100644 --- a/llvm/unittests/TextAPI/TextStubV1Tests.cpp +++ b/llvm/unittests/TextAPI/TextStubV1Tests.cpp @@ -451,8 +451,8 @@ TEST(TBDv1, MalformedFile2) { 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); } diff --git a/llvm/unittests/TextAPI/TextStubV2Tests.cpp b/llvm/unittests/TextAPI/TextStubV2Tests.cpp index 71d4397..c9e54ac 100644 --- a/llvm/unittests/TextAPI/TextStubV2Tests.cpp +++ b/llvm/unittests/TextAPI/TextStubV2Tests.cpp @@ -486,8 +486,8 @@ TEST(TBDv2, MalformedFile2) { 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); } diff --git a/llvm/unittests/TextAPI/TextStubV3Tests.cpp b/llvm/unittests/TextAPI/TextStubV3Tests.cpp index a6a88c4..2881c95 100644 --- a/llvm/unittests/TextAPI/TextStubV3Tests.cpp +++ b/llvm/unittests/TextAPI/TextStubV3Tests.cpp @@ -831,8 +831,8 @@ TEST(TBDv3, MalformedFile2) { 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); } diff --git a/llvm/unittests/TextAPI/TextStubV4Tests.cpp b/llvm/unittests/TextAPI/TextStubV4Tests.cpp index 43b53e1..0ab9c52 100644 --- a/llvm/unittests/TextAPI/TextStubV4Tests.cpp +++ b/llvm/unittests/TextAPI/TextStubV4Tests.cpp @@ -924,8 +924,8 @@ TEST(TBDv4, MalformedFile2) { 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); } -- 2.7.4