Skip to content

Commit

Permalink
[ASTMatchers] Matchers that take enumerations args provide hints with…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
njames93 committed Apr 6, 2020
1 parent 53b7abd commit 427c1dc
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 5 deletions.
1 change: 1 addition & 0 deletions clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
Expand Up @@ -65,6 +65,7 @@ class Diagnostics {
ET_RegistryNotBindable = 4,
ET_RegistryAmbiguousOverload = 5,
ET_RegistryValueNotFound = 6,
ET_RegistryUnknownEnumWithReplace = 7,

ET_ParserStringError = 100,
ET_ParserNoOpenParen = 101,
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
Expand Up @@ -14,9 +14,10 @@ endif()

add_clang_library(clangDynamicASTMatchers
Diagnostics.cpp
VariantValue.cpp
Marshallers.cpp
Parser.cpp
Registry.cpp
VariantValue.cpp

LINK_LIBS
clangAST
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
Expand Up @@ -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>";
Expand Down
89 changes: 89 additions & 0 deletions 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;
}
42 changes: 38 additions & 4 deletions clang/lib/ASTMatchers/Dynamic/Marshallers.h
Expand Up @@ -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"
Expand Down Expand Up @@ -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 <>
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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.
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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(); \
}

Expand Down
14 changes: 14 additions & 0 deletions clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
Expand Up @@ -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) {
Expand Down

0 comments on commit 427c1dc

Please sign in to comment.