From fd1323498d2cd38d5a6d8bc679bdbd48a8a2c534 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Sat, 23 Nov 2013 01:13:16 +0000 Subject: [PATCH] Split registry matcher resolution into a lookup phase and a construction phase. The looked-up matchers will be used during code completion. Differential Revision: http://llvm-reviews.chandlerc.com/D2207 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@195534 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/ASTMatchers/Dynamic/Parser.h | 19 ++++++++++-- include/clang/ASTMatchers/Dynamic/Registry.h | 31 +++++++++++++------ lib/ASTMatchers/Dynamic/Parser.cpp | 22 ++++++++++--- lib/ASTMatchers/Dynamic/Registry.cpp | 23 +++++++++----- unittests/ASTMatchers/Dynamic/ParserTest.cpp | 28 ++++++++++++----- .../ASTMatchers/Dynamic/RegistryTest.cpp | 29 +++++++++++++---- 6 files changed, 115 insertions(+), 37 deletions(-) diff --git a/include/clang/ASTMatchers/Dynamic/Parser.h b/include/clang/ASTMatchers/Dynamic/Parser.h index bb6ac76989..05c197fadc 100644 --- a/include/clang/ASTMatchers/Dynamic/Parser.h +++ b/include/clang/ASTMatchers/Dynamic/Parser.h @@ -34,6 +34,7 @@ #define LLVM_CLANG_AST_MATCHERS_DYNAMIC_PARSER_H #include "clang/ASTMatchers/Dynamic/Diagnostics.h" +#include "clang/ASTMatchers/Dynamic/Registry.h" #include "clang/ASTMatchers/Dynamic/VariantValue.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/ArrayRef.h" @@ -65,7 +66,7 @@ public: /// /// All the arguments passed here have already been processed. /// - /// \param MatcherName The matcher name found by the parser. + /// \param Ctor A matcher constructor looked up by lookupMatcherCtor. /// /// \param NameRange The location of the name in the matcher source. /// Useful for error reporting. @@ -78,11 +79,25 @@ public: /// \return The matcher objects constructed by the processor, or a null /// matcher if an error occurred. In that case, \c Error will contain a /// description of the error. - virtual VariantMatcher actOnMatcherExpression(StringRef MatcherName, + virtual VariantMatcher actOnMatcherExpression(MatcherCtor Ctor, const SourceRange &NameRange, StringRef BindID, ArrayRef Args, Diagnostics *Error) = 0; + + /// \brief Look up a matcher by name. + /// + /// \param MatcherName The matcher name found by the parser. + /// + /// \param NameRange The location of the name in the matcher source. + /// Useful for error reporting. + /// + /// \return The matcher constructor, or Optional() if an error + /// occurred. In that case, \c Error will contain a description of the + /// error. + virtual llvm::Optional + lookupMatcherCtor(StringRef MatcherName, const SourceRange &NameRange, + Diagnostics *Error) = 0; }; /// \brief Parse a matcher expression, creating matchers from the registry. diff --git a/include/clang/ASTMatchers/Dynamic/Registry.h b/include/clang/ASTMatchers/Dynamic/Registry.h index c113c1404e..ffdd8b21a9 100644 --- a/include/clang/ASTMatchers/Dynamic/Registry.h +++ b/include/clang/ASTMatchers/Dynamic/Registry.h @@ -21,20 +21,33 @@ #include "clang/ASTMatchers/Dynamic/VariantValue.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" namespace clang { namespace ast_matchers { namespace dynamic { +namespace internal { +class MatcherCreateCallback; +} + +typedef const internal::MatcherCreateCallback *MatcherCtor; + class Registry { public: - /// \brief Construct a matcher from the registry by name. + /// \brief Look up a matcher in the registry by name, /// - /// Consult the registry of known matchers and construct the appropriate - /// matcher by name. + /// \return An opaque value which may be used to refer to the matcher + /// constructor, or Optional() if not found. In that case + /// \c Error will contain the description of the error. + static llvm::Optional + lookupMatcherCtor(StringRef MatcherName, const SourceRange &NameRange, + Diagnostics *Error); + + /// \brief Construct a matcher from the registry. /// - /// \param MatcherName The name of the matcher to instantiate. + /// \param Ctor The matcher constructor to instantiate. /// /// \param NameRange The location of the name in the matcher source. /// Useful for error reporting. @@ -44,10 +57,10 @@ public: /// will return an error. /// /// \return The matcher object constructed if no error was found. - /// A null matcher if the matcher is not found, or if the number of - /// arguments or argument types do not match the signature. - /// In that case \c Error will contain the description of the error. - static VariantMatcher constructMatcher(StringRef MatcherName, + /// A null matcher if the number of arguments or argument types do not match + /// the signature. In that case \c Error will contain the description of + /// the error. + static VariantMatcher constructMatcher(MatcherCtor Ctor, const SourceRange &NameRange, ArrayRef Args, Diagnostics *Error); @@ -58,7 +71,7 @@ public: /// matcher to the specified \c BindID. /// If the matcher is not bindable, it sets an error in \c Error and returns /// a null matcher. - static VariantMatcher constructBoundMatcher(StringRef MatcherName, + static VariantMatcher constructBoundMatcher(MatcherCtor Ctor, const SourceRange &NameRange, StringRef BindID, ArrayRef Args, diff --git a/lib/ASTMatchers/Dynamic/Parser.cpp b/lib/ASTMatchers/Dynamic/Parser.cpp index df9596e9b9..59a75479ef 100644 --- a/lib/ASTMatchers/Dynamic/Parser.cpp +++ b/lib/ASTMatchers/Dynamic/Parser.cpp @@ -18,6 +18,7 @@ #include "clang/ASTMatchers/Dynamic/Parser.h" #include "clang/ASTMatchers/Dynamic/Registry.h" #include "clang/Basic/CharInfo.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/Twine.h" namespace clang { @@ -242,6 +243,9 @@ bool Parser::parseMatcherExpressionImpl(VariantValue *Value) { return false; } + llvm::Optional Ctor = + S->lookupMatcherCtor(NameToken.Text, NameToken.Range, Error); + std::vector Args; TokenInfo EndToken; while (Tokenizer->nextTokenKind() != TokenInfo::TK_Eof) { @@ -306,13 +310,16 @@ bool Parser::parseMatcherExpressionImpl(VariantValue *Value) { BindID = IDToken.Value.getString(); } + if (!Ctor) + return false; + // Merge the start and end infos. Diagnostics::Context Ctx(Diagnostics::Context::ConstructMatcher, Error, NameToken.Text, NameToken.Range); SourceRange MatcherRange = NameToken.Range; MatcherRange.End = EndToken.Range.End; VariantMatcher Result = S->actOnMatcherExpression( - NameToken.Text, MatcherRange, BindID, Args, Error); + *Ctor, MatcherRange, BindID, Args, Error); if (Result.isNull()) return false; *Value = Result; @@ -358,16 +365,21 @@ Parser::Parser(CodeTokenizer *Tokenizer, Sema *S, class RegistrySema : public Parser::Sema { public: virtual ~RegistrySema() {} - VariantMatcher actOnMatcherExpression(StringRef MatcherName, + llvm::Optional lookupMatcherCtor(StringRef MatcherName, + const SourceRange &NameRange, + Diagnostics *Error) { + return Registry::lookupMatcherCtor(MatcherName, NameRange, Error); + } + VariantMatcher actOnMatcherExpression(MatcherCtor Ctor, const SourceRange &NameRange, StringRef BindID, ArrayRef Args, Diagnostics *Error) { if (BindID.empty()) { - return Registry::constructMatcher(MatcherName, NameRange, Args, Error); + return Registry::constructMatcher(Ctor, NameRange, Args, Error); } else { - return Registry::constructBoundMatcher(MatcherName, NameRange, BindID, - Args, Error); + return Registry::constructBoundMatcher(Ctor, NameRange, BindID, Args, + Error); } } }; diff --git a/lib/ASTMatchers/Dynamic/Registry.cpp b/lib/ASTMatchers/Dynamic/Registry.cpp index a19cdc06e9..40dd1b620b 100644 --- a/lib/ASTMatchers/Dynamic/Registry.cpp +++ b/lib/ASTMatchers/Dynamic/Registry.cpp @@ -306,27 +306,34 @@ static llvm::ManagedStatic RegistryData; } // anonymous namespace // static -VariantMatcher Registry::constructMatcher(StringRef MatcherName, - const SourceRange &NameRange, - ArrayRef Args, - Diagnostics *Error) { +llvm::Optional +Registry::lookupMatcherCtor(StringRef MatcherName, const SourceRange &NameRange, + Diagnostics *Error) { ConstructorMap::const_iterator it = RegistryData->constructors().find(MatcherName); if (it == RegistryData->constructors().end()) { Error->addError(NameRange, Error->ET_RegistryNotFound) << MatcherName; - return VariantMatcher(); + return llvm::Optional(); } - return it->second->run(NameRange, Args, Error); + return it->second; +} + +// static +VariantMatcher Registry::constructMatcher(MatcherCtor Ctor, + const SourceRange &NameRange, + ArrayRef Args, + Diagnostics *Error) { + return Ctor->run(NameRange, Args, Error); } // static -VariantMatcher Registry::constructBoundMatcher(StringRef MatcherName, +VariantMatcher Registry::constructBoundMatcher(MatcherCtor Ctor, const SourceRange &NameRange, StringRef BindID, ArrayRef Args, Diagnostics *Error) { - VariantMatcher Out = constructMatcher(MatcherName, NameRange, Args, Error); + VariantMatcher Out = constructMatcher(Ctor, NameRange, Args, Error); if (Out.isNull()) return Out; llvm::Optional Result = Out.getSingleMatcher(); diff --git a/unittests/ASTMatchers/Dynamic/ParserTest.cpp b/unittests/ASTMatchers/Dynamic/ParserTest.cpp index f19ec51ad7..dc3fa24252 100644 --- a/unittests/ASTMatchers/Dynamic/ParserTest.cpp +++ b/unittests/ASTMatchers/Dynamic/ParserTest.cpp @@ -14,6 +14,7 @@ #include "clang/ASTMatchers/Dynamic/Parser.h" #include "clang/ASTMatchers/Dynamic/Registry.h" #include "gtest/gtest.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringMap.h" namespace clang { @@ -39,15 +40,24 @@ public: Errors.push_back(Error.toStringFull()); } - VariantMatcher actOnMatcherExpression(StringRef MatcherName, + llvm::Optional lookupMatcherCtor(StringRef MatcherName, + const SourceRange &NameRange, + Diagnostics *Error) { + const ExpectedMatchersTy::value_type *Matcher = + &*ExpectedMatchers.find(MatcherName); + return reinterpret_cast(Matcher); + } + + VariantMatcher actOnMatcherExpression(MatcherCtor Ctor, const SourceRange &NameRange, StringRef BindID, ArrayRef Args, Diagnostics *Error) { - MatcherInfo ToStore = { MatcherName, NameRange, Args, BindID }; + const ExpectedMatchersTy::value_type *Matcher = + reinterpret_cast(Ctor); + MatcherInfo ToStore = { Matcher->first, NameRange, Args, BindID }; Matchers.push_back(ToStore); - return VariantMatcher::SingleMatcher( - ExpectedMatchers.find(MatcherName)->second); + return VariantMatcher::SingleMatcher(Matcher->second); } struct MatcherInfo { @@ -60,8 +70,9 @@ public: std::vector Errors; std::vector Values; std::vector Matchers; - std::map > - ExpectedMatchers; + typedef std::map > + ExpectedMatchersTy; + ExpectedMatchersTy ExpectedMatchers; }; TEST(ParserTest, ParseUnsigned) { @@ -194,16 +205,19 @@ TEST(ParserTest, Errors) { "1:5: Error parsing matcher. Found token <123> while looking for '('.", ParseWithError("Foo 123")); EXPECT_EQ( + "1:1: Matcher not found: Foo\n" "1:9: Error parsing matcher. Found token <123> while looking for ','.", ParseWithError("Foo(\"A\" 123)")); EXPECT_EQ( + "1:1: Matcher not found: Foo\n" "1:4: Error parsing matcher. Found end-of-code while looking for ')'.", ParseWithError("Foo(")); EXPECT_EQ("1:1: End of code found while looking for token.", ParseWithError("")); EXPECT_EQ("Input value is not a matcher expression.", ParseMatcherWithError("\"A\"")); - EXPECT_EQ("1:1: Error parsing argument 1 for matcher Foo.\n" + EXPECT_EQ("1:1: Matcher not found: Foo\n" + "1:1: Error parsing argument 1 for matcher Foo.\n" "1:5: Invalid token <(> found when looking for a value.", ParseWithError("Foo((")); EXPECT_EQ("1:7: Expected end of code.", ParseWithError("expr()a")); diff --git a/unittests/ASTMatchers/Dynamic/RegistryTest.cpp b/unittests/ASTMatchers/Dynamic/RegistryTest.cpp index 8694c642cd..5f3079017a 100644 --- a/unittests/ASTMatchers/Dynamic/RegistryTest.cpp +++ b/unittests/ASTMatchers/Dynamic/RegistryTest.cpp @@ -36,12 +36,24 @@ public: return Out; } + llvm::Optional lookupMatcherCtor(StringRef MatcherName, + Diagnostics *Error = 0) { + Diagnostics DummyError; + if (!Error) Error = &DummyError; + llvm::Optional Ctor = + Registry::lookupMatcherCtor(MatcherName, SourceRange(), Error); + EXPECT_EQ("", DummyError.toStringFull()); + return Ctor; + } + VariantMatcher constructMatcher(StringRef MatcherName, Diagnostics *Error = NULL) { Diagnostics DummyError; if (!Error) Error = &DummyError; - const VariantMatcher Out = - Registry::constructMatcher(MatcherName, SourceRange(), Args(), Error); + llvm::Optional Ctor = lookupMatcherCtor(MatcherName, Error); + VariantMatcher Out; + if (Ctor) + Out = Registry::constructMatcher(*Ctor, SourceRange(), Args(), Error); EXPECT_EQ("", DummyError.toStringFull()); return Out; } @@ -51,8 +63,10 @@ public: Diagnostics *Error = NULL) { Diagnostics DummyError; if (!Error) Error = &DummyError; - const VariantMatcher Out = Registry::constructMatcher( - MatcherName, SourceRange(), Args(Arg1), Error); + llvm::Optional Ctor = lookupMatcherCtor(MatcherName, Error); + VariantMatcher Out; + if (Ctor) + Out = Registry::constructMatcher(*Ctor, SourceRange(), Args(Arg1), Error); EXPECT_EQ("", DummyError.toStringFull()); return Out; } @@ -63,8 +77,11 @@ public: Diagnostics *Error = NULL) { Diagnostics DummyError; if (!Error) Error = &DummyError; - const VariantMatcher Out = Registry::constructMatcher( - MatcherName, SourceRange(), Args(Arg1, Arg2), Error); + llvm::Optional Ctor = lookupMatcherCtor(MatcherName, Error); + VariantMatcher Out; + if (Ctor) + Out = Registry::constructMatcher(*Ctor, SourceRange(), Args(Arg1, Arg2), + Error); EXPECT_EQ("", DummyError.toStringFull()); return Out; } -- 2.40.0