diff --git a/clang/lib/ASTMatchers/Dynamic/Marshallers.h b/clang/lib/ASTMatchers/Dynamic/Marshallers.h index 33f6d1e4155c..9758d36b0dc6 100644 --- a/clang/lib/ASTMatchers/Dynamic/Marshallers.h +++ b/clang/lib/ASTMatchers/Dynamic/Marshallers.h @@ -58,7 +58,10 @@ template struct ArgTypeTraits : public ArgTypeTraits { }; template <> struct ArgTypeTraits { - static bool is(const VariantValue &Value) { return Value.isString(); } + static bool hasCorrectType(const VariantValue &Value) { + return Value.isString(); + } + static bool hasCorrectValue(const VariantValue &Value) { return true; } static const std::string &get(const VariantValue &Value) { return Value.getString(); @@ -78,8 +81,11 @@ struct ArgTypeTraits : public ArgTypeTraits { }; template struct ArgTypeTraits> { - static bool is(const VariantValue &Value) { - return Value.isMatcher() && Value.getMatcher().hasTypedMatcher(); + static bool hasCorrectType(const VariantValue& Value) { + return Value.isMatcher(); + } + static bool hasCorrectValue(const VariantValue &Value) { + return Value.getMatcher().hasTypedMatcher(); } static ast_matchers::internal::Matcher get(const VariantValue &Value) { @@ -96,7 +102,10 @@ template struct ArgTypeTraits> { }; template <> struct ArgTypeTraits { - static bool is(const VariantValue &Value) { return Value.isBoolean(); } + static bool hasCorrectType(const VariantValue &Value) { + return Value.isBoolean(); + } + static bool hasCorrectValue(const VariantValue &Value) { return true; } static bool get(const VariantValue &Value) { return Value.getBoolean(); @@ -112,7 +121,10 @@ template <> struct ArgTypeTraits { }; template <> struct ArgTypeTraits { - static bool is(const VariantValue &Value) { return Value.isDouble(); } + static bool hasCorrectType(const VariantValue &Value) { + return Value.isDouble(); + } + static bool hasCorrectValue(const VariantValue &Value) { return true; } static double get(const VariantValue &Value) { return Value.getDouble(); @@ -128,7 +140,10 @@ template <> struct ArgTypeTraits { }; template <> struct ArgTypeTraits { - static bool is(const VariantValue &Value) { return Value.isUnsigned(); } + static bool hasCorrectType(const VariantValue &Value) { + return Value.isUnsigned(); + } + static bool hasCorrectValue(const VariantValue &Value) { return true; } static unsigned get(const VariantValue &Value) { return Value.getUnsigned(); @@ -153,8 +168,11 @@ private: } public: - static bool is(const VariantValue &Value) { - return Value.isString() && getAttrKind(Value.getString()); + static bool hasCorrectType(const VariantValue &Value) { + return Value.isString(); + } + static bool hasCorrectValue(const VariantValue& Value) { + return getAttrKind(Value.getString()).hasValue(); } static attr::Kind get(const VariantValue &Value) { @@ -178,8 +196,11 @@ private: } public: - static bool is(const VariantValue &Value) { - return Value.isString() && getCastKind(Value.getString()); + static bool hasCorrectType(const VariantValue &Value) { + return Value.isString(); + } + static bool hasCorrectValue(const VariantValue& Value) { + return getCastKind(Value.getString()).hasValue(); } static CastKind get(const VariantValue &Value) { @@ -198,8 +219,11 @@ private: static Optional getFlags(llvm::StringRef Flags); public: - static bool is(const VariantValue &Value) { - return Value.isString() && getFlags(Value.getString()); + static bool hasCorrectType(const VariantValue &Value) { + return Value.isString(); + } + static bool hasCorrectValue(const VariantValue& Value) { + return getFlags(Value.getString()).hasValue(); } static llvm::Regex::RegexFlags get(const VariantValue &Value) { @@ -221,8 +245,11 @@ private: } public: - static bool is(const VariantValue &Value) { - return Value.isString() && getClauseKind(Value.getString()); + static bool hasCorrectType(const VariantValue &Value) { + return Value.isString(); + } + static bool hasCorrectValue(const VariantValue& Value) { + return getClauseKind(Value.getString()).hasValue(); } static OpenMPClauseKind get(const VariantValue &Value) { @@ -248,8 +275,11 @@ private: } public: - static bool is(const VariantValue &Value) { - return Value.isString() && getUnaryOrTypeTraitKind(Value.getString()); + static bool hasCorrectType(const VariantValue &Value) { + return Value.isString(); + } + static bool hasCorrectValue(const VariantValue& Value) { + return getUnaryOrTypeTraitKind(Value.getString()).hasValue(); } static UnaryExprOrTypeTrait get(const VariantValue &Value) { @@ -453,12 +483,31 @@ variadicMatcherDescriptor(StringRef MatcherName, SourceRange NameRange, const ParserValue &Arg = Args[i]; const VariantValue &Value = Arg.Value; - if (!ArgTraits::is(Value)) { + if (!ArgTraits::hasCorrectType(Value)) { Error->addError(Arg.Range, Error->ET_RegistryWrongArgType) << (i + 1) << ArgTraits::getKind().asString() << Value.getTypeAsString(); HasError = true; break; } + if (!ArgTraits::hasCorrectValue(Value)) { + if (llvm::Optional BestGuess = + ArgTraits::getBestGuess(Value)) { + Error->addError(Arg.Range, Error->ET_RegistryUnknownEnumWithReplace) + << i + 1 << Value.getString() << *BestGuess; + } else if (Value.isString()) { + Error->addError(Arg.Range, Error->ET_RegistryValueNotFound) + << Value.getString(); + } else { + // This isn't ideal, but it's better than reporting an empty string as + // the error in this case. + Error->addError(Arg.Range, Error->ET_RegistryWrongArgType) + << (i + 1) << ArgTraits::getKind().asString() + << Value.getTypeAsString(); + } + HasError = true; + break; + } + InnerArgs[i] = new ArgT(ArgTraits::get(Value)); } @@ -568,16 +617,21 @@ private: } #define CHECK_ARG_TYPE(index, type) \ - if (!ArgTypeTraits::is(Args[index].Value)) { \ + if (!ArgTypeTraits::hasCorrectType(Args[index].Value)) { \ + Error->addError(Args[index].Range, Error->ET_RegistryWrongArgType) \ + << (index + 1) << ArgTypeTraits::getKind().asString() \ + << Args[index].Value.getTypeAsString(); \ + return VariantMatcher(); \ + } \ + if (!ArgTypeTraits::hasCorrectValue(Args[index].Value)) { \ if (llvm::Optional BestGuess = \ ArgTypeTraits::getBestGuess(Args[index].Value)) { \ Error->addError(Args[index].Range, \ Error->ET_RegistryUnknownEnumWithReplace) \ << index + 1 << Args[index].Value.getString() << *BestGuess; \ - } else { \ - Error->addError(Args[index].Range, Error->ET_RegistryWrongArgType) \ - << (index + 1) << ArgTypeTraits::getKind().asString() \ - << Args[index].Value.getTypeAsString(); \ + } else if (Args[index].Value.isString()) { \ + Error->addError(Args[index].Range, Error->ET_RegistryValueNotFound) \ + << Args[index].Value.getString(); \ } \ return VariantMatcher(); \ } @@ -761,7 +815,7 @@ public: << "1 or 2" << Args.size(); return VariantMatcher(); } - if (!ArgTypeTraits::is(Args[0].Value)) { + if (!ArgTypeTraits::hasCorrectType(Args[0].Value)) { Error->addError(Args[0].Range, Error->ET_RegistryWrongArgType) << 1 << ArgTypeTraits::getKind().asString() << Args[0].Value.getTypeAsString(); @@ -771,16 +825,23 @@ public: return outvalueToVariantMatcher( NoFlags(ArgTypeTraits::get(Args[0].Value))); } - if (!ArgTypeTraits::is(Args[1].Value)) { + if (!ArgTypeTraits::hasCorrectType( + Args[1].Value)) { + Error->addError(Args[1].Range, Error->ET_RegistryWrongArgType) + << 2 << ArgTypeTraits::getKind().asString() + << Args[1].Value.getTypeAsString(); + return VariantMatcher(); + } + if (!ArgTypeTraits::hasCorrectValue( + Args[1].Value)) { if (llvm::Optional BestGuess = ArgTypeTraits::getBestGuess( Args[1].Value)) { Error->addError(Args[1].Range, Error->ET_RegistryUnknownEnumWithReplace) << 2 << Args[1].Value.getString() << *BestGuess; } else { - Error->addError(Args[1].Range, Error->ET_RegistryWrongArgType) - << 2 << ArgTypeTraits::getKind().asString() - << Args[1].Value.getTypeAsString(); + Error->addError(Args[1].Range, Error->ET_RegistryValueNotFound) + << Args[1].Value.getString(); } return VariantMatcher(); } diff --git a/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp b/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp index 3af5574216f1..6fc720ed982f 100644 --- a/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp +++ b/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp @@ -354,8 +354,7 @@ TEST(ParserTest, Errors) { ParseMatcherWithError(R"query(decl(hasAttr("Final")))query")); EXPECT_EQ("1:1: Error parsing argument 1 for matcher decl.\n" "1:6: Error building matcher hasAttr.\n" - "1:14: Incorrect type for arg 1. (Expected = string) != (Actual = " - "String)", + "1:14: Value not found: unrelated", ParseMatcherWithError(R"query(decl(hasAttr("unrelated")))query")); EXPECT_EQ( "1:1: Error parsing argument 1 for matcher namedDecl.\n" @@ -366,8 +365,7 @@ TEST(ParserTest, Errors) { EXPECT_EQ( "1:1: Error parsing argument 1 for matcher namedDecl.\n" "1:11: Error building matcher matchesName.\n" - "1:33: Incorrect type for arg 2. (Expected = string) != (Actual = " - "String)", + "1:33: Value not found: IgnoreCase & BasicRegex", ParseMatcherWithError( R"query(namedDecl(matchesName("[ABC]*", "IgnoreCase & BasicRegex")))query")); EXPECT_EQ(