[clang] 427c1dc - [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 12:00:38 PDT 2020
Author: Nathan James
Date: 2020-04-06T20:00:30+01:00
New Revision: 427c1dc4f4248ec71e32a8f3832a4e6258446672
URL: https://github.com/llvm/llvm-project/commit/427c1dc4f4248ec71e32a8f3832a4e6258446672
DIFF: https://github.com/llvm/llvm-project/commit/427c1dc4f4248ec71e32a8f3832a4e6258446672.diff
LOG: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments
Summary:
This adds support for giving hints when using dynamic matchers with enum args. Take this query, I couldn't figure out why the matcher wasn't working:
(Turns out it needed to be "IntegralToBoolean", but thats another bug itself)
```
clang-query> match implicitCastExpr(hasCastKind("CK_IntegralToBoolean"))
1:1: Error parsing argument 1 for matcher implicitCastExpr.
1:18: Error building matcher hasCastKind.
1:30: Incorrect type for arg 1. (Expected = string) != (Actual = String)
```
With this patch the new behaviour looks like this:
```
clang-query> match implicitCastExpr(hasCastKind("CK_IntegralToBoolean"))
1:1: Error parsing argument 1 for matcher implicitCastExpr.
1:18: Error building matcher hasCastKind.
1:30: Unknown value 'CK_IntegralToBoolean' for arg 1; did you mean 'IntegralToBoolean'
```
There are no test cases for this yet as there simply isn't any infrastructure for testing errors reported when parsing args that I can see.
Reviewers: klimek, jdoerfert, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: aaron.ballman, dexonsmith, mgorny, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77499
Added:
clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
Modified:
clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
clang/lib/ASTMatchers/Dynamic/Marshallers.h
clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h b/clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
index 7dd304797c4f..f095dcdd60b0 100644
--- a/clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
+++ b/clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
@@ -65,6 +65,7 @@ class Diagnostics {
ET_RegistryNotBindable = 4,
ET_RegistryAmbiguousOverload = 5,
ET_RegistryValueNotFound = 6,
+ ET_RegistryUnknownEnumWithReplace = 7,
ET_ParserStringError = 100,
ET_ParserNoOpenParen = 101,
diff --git a/clang/lib/ASTMatchers/Dynamic/CMakeLists.txt b/clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
index 10bd9c455eb4..1c6c1e68ce1e 100644
--- a/clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
+++ b/clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
@@ -14,9 +14,10 @@ endif()
add_clang_library(clangDynamicASTMatchers
Diagnostics.cpp
- VariantValue.cpp
+ Marshallers.cpp
Parser.cpp
Registry.cpp
+ VariantValue.cpp
LINK_LIBS
clangAST
diff --git a/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp b/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
index 8656bca870ec..969bd5f31789 100644
--- a/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
@@ -98,6 +98,8 @@ static StringRef errorTypeToFormatString(Diagnostics::ErrorType Type) {
return "Ambiguous matcher overload.";
case Diagnostics::ET_RegistryValueNotFound:
return "Value not found: $0";
+ case Diagnostics::ET_RegistryUnknownEnumWithReplace:
+ return "Unknown value '$1' for arg $0; did you mean '$2'";
case Diagnostics::ET_ParserStringError:
return "Error parsing string token: <$0>";
diff --git a/clang/lib/ASTMatchers/Dynamic/Marshallers.cpp b/clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
new file mode 100644
index 000000000000..7e4e3ceb2b41
--- /dev/null
+++ b/clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
@@ -0,0 +1,89 @@
+#include "Marshallers.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include <string>
+
+static llvm::Optional<std::string>
+getBestGuess(llvm::StringRef Search, llvm::ArrayRef<llvm::StringRef> Allowed,
+ llvm::StringRef DropPrefix = "", unsigned MaxEditDistance = 3) {
+ if (MaxEditDistance != ~0U)
+ ++MaxEditDistance;
+ llvm::StringRef Res;
+ for (const llvm::StringRef &Item : Allowed) {
+ if (Item.equals_lower(Search)) {
+ assert(!Item.equals(Search) && "This should be handled earlier on.");
+ MaxEditDistance = 1;
+ Res = Item;
+ continue;
+ }
+ unsigned Distance = Item.edit_distance(Search);
+ if (Distance < MaxEditDistance) {
+ MaxEditDistance = Distance;
+ Res = Item;
+ }
+ }
+ if (!Res.empty())
+ return Res.str();
+ if (!DropPrefix.empty()) {
+ --MaxEditDistance; // Treat dropping the prefix as 1 edit
+ for (const llvm::StringRef &Item : Allowed) {
+ auto NoPrefix = Item;
+ if (!NoPrefix.consume_front(DropPrefix))
+ continue;
+ if (NoPrefix.equals_lower(Search)) {
+ if (NoPrefix.equals(Search))
+ return Item.str();
+ MaxEditDistance = 1;
+ Res = Item;
+ continue;
+ }
+ unsigned Distance = NoPrefix.edit_distance(Search);
+ if (Distance < MaxEditDistance) {
+ MaxEditDistance = Distance;
+ Res = Item;
+ }
+ }
+ if (!Res.empty())
+ return Res.str();
+ }
+ return llvm::None;
+}
+
+llvm::Optional<std::string>
+clang::ast_matchers::dynamic::internal::ArgTypeTraits<
+ clang::attr::Kind>::getBestGuess(const VariantValue &Value) {
+ static constexpr llvm::StringRef Allowed[] = {
+#define ATTR(X) "attr::" #X,
+#include "clang/Basic/AttrList.inc"
+ };
+ if (Value.isString())
+ return ::getBestGuess(Value.getString(), llvm::makeArrayRef(Allowed),
+ "attr::");
+ return llvm::None;
+}
+
+llvm::Optional<std::string>
+clang::ast_matchers::dynamic::internal::ArgTypeTraits<
+ clang::CastKind>::getBestGuess(const VariantValue &Value) {
+ static constexpr llvm::StringRef Allowed[] = {
+#define CAST_OPERATION(Name) #Name,
+#include "clang/AST/OperationKinds.def"
+ };
+ if (Value.isString())
+ return ::getBestGuess(Value.getString(), llvm::makeArrayRef(Allowed));
+ return llvm::None;
+}
+
+llvm::Optional<std::string>
+clang::ast_matchers::dynamic::internal::ArgTypeTraits<
+ clang::OpenMPClauseKind>::getBestGuess(const VariantValue &Value) {
+ static constexpr llvm::StringRef Allowed[] = {
+#define OPENMP_CLAUSE(TextualSpelling, Class) "OMPC_" #TextualSpelling,
+#include "clang/Basic/OpenMPKinds.def"
+ };
+ if (Value.isString())
+ return ::getBestGuess(Value.getString(), llvm::makeArrayRef(Allowed),
+ "OMPC_");
+ return llvm::None;
+}
diff --git a/clang/lib/ASTMatchers/Dynamic/Marshallers.h b/clang/lib/ASTMatchers/Dynamic/Marshallers.h
index db9122acede7..3d36ed6ea187 100644
--- a/clang/lib/ASTMatchers/Dynamic/Marshallers.h
+++ b/clang/lib/ASTMatchers/Dynamic/Marshallers.h
@@ -29,6 +29,7 @@
#include "clang/Basic/OpenMPKinds.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
@@ -64,6 +65,10 @@ template <> struct ArgTypeTraits<std::string> {
static ArgKind getKind() {
return ArgKind(ArgKind::AK_String);
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &) {
+ return llvm::None;
+ }
};
template <>
@@ -82,6 +87,10 @@ template <class T> struct ArgTypeTraits<ast_matchers::internal::Matcher<T>> {
static ArgKind getKind() {
return ArgKind(ASTNodeKind::getFromNodeKind<T>());
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &) {
+ return llvm::None;
+ }
};
template <> struct ArgTypeTraits<bool> {
@@ -94,6 +103,10 @@ template <> struct ArgTypeTraits<bool> {
static ArgKind getKind() {
return ArgKind(ArgKind::AK_Boolean);
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &) {
+ return llvm::None;
+ }
};
template <> struct ArgTypeTraits<double> {
@@ -106,6 +119,10 @@ template <> struct ArgTypeTraits<double> {
static ArgKind getKind() {
return ArgKind(ArgKind::AK_Double);
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &) {
+ return llvm::None;
+ }
};
template <> struct ArgTypeTraits<unsigned> {
@@ -118,6 +135,10 @@ template <> struct ArgTypeTraits<unsigned> {
static ArgKind getKind() {
return ArgKind(ArgKind::AK_Unsigned);
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &) {
+ return llvm::None;
+ }
};
template <> struct ArgTypeTraits<attr::Kind> {
@@ -141,6 +162,8 @@ template <> struct ArgTypeTraits<attr::Kind> {
static ArgKind getKind() {
return ArgKind(ArgKind::AK_String);
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &Value);
};
template <> struct ArgTypeTraits<CastKind> {
@@ -164,6 +187,8 @@ template <> struct ArgTypeTraits<CastKind> {
static ArgKind getKind() {
return ArgKind(ArgKind::AK_String);
}
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &Value);
};
template <> struct ArgTypeTraits<OpenMPClauseKind> {
@@ -185,6 +210,8 @@ template <> struct ArgTypeTraits<OpenMPClauseKind> {
}
static ArgKind getKind() { return ArgKind(ArgKind::AK_String); }
+
+ static llvm::Optional<std::string> getBestGuess(const VariantValue &Value);
};
/// Matcher descriptor interface.
@@ -318,7 +345,7 @@ static void mergePolyMatchers(const PolyMatcher &Poly,
/// polymorphic matcher. For the former, we just construct the VariantMatcher.
/// For the latter, we instantiate all the possible Matcher<T> of the poly
/// matcher.
-static VariantMatcher outvalueToVariantMatcher(const DynTypedMatcher &Matcher) {
+inline VariantMatcher outvalueToVariantMatcher(const DynTypedMatcher &Matcher) {
return VariantMatcher::SingleMatcher(Matcher);
}
@@ -495,9 +522,16 @@ class DynCastAllOfMatcherDescriptor : public VariadicFuncMatcherDescriptor {
#define CHECK_ARG_TYPE(index, type) \
if (!ArgTypeTraits<type>::is(Args[index].Value)) { \
- Error->addError(Args[index].Range, Error->ET_RegistryWrongArgType) \
- << (index + 1) << ArgTypeTraits<type>::getKind().asString() \
- << Args[index].Value.getTypeAsString(); \
+ if (llvm::Optional<std::string> BestGuess = \
+ ArgTypeTraits<type>::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<type>::getKind().asString() \
+ << Args[index].Value.getTypeAsString(); \
+ } \
return VariantMatcher(); \
}
diff --git a/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp b/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
index 41ef0d848963..71c2e3e5addf 100644
--- a/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ b/clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -317,6 +317,20 @@ TEST(ParserTest, Errors) {
EXPECT_EQ("Input value has unresolved overloaded type: "
"Matcher<DoStmt|ForStmt|WhileStmt|CXXForRangeStmt|FunctionDecl>",
ParseMatcherWithError("hasBody(stmt())"));
+ EXPECT_EQ(
+ "1:1: Error parsing argument 1 for matcher decl.\n"
+ "1:6: Error building matcher hasAttr.\n"
+ "1:14: Unknown value 'attr::Fnal' for arg 1; did you mean 'attr::Final'",
+ ParseMatcherWithError(R"query(decl(hasAttr("attr::Fnal")))query"));
+ EXPECT_EQ("1:1: Error parsing argument 1 for matcher decl.\n"
+ "1:6: Error building matcher hasAttr.\n"
+ "1:14: Unknown value 'Final' for arg 1; did you mean 'attr::Final'",
+ 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)",
+ ParseMatcherWithError(R"query(decl(hasAttr("unrelated")))query"));
}
TEST(ParserTest, OverloadErrors) {
More information about the cfe-commits
mailing list