Details
Diff Detail
- Build Status
Buildable 1760 Build 1760: arc lint + arc unit
Event Timeline
I've tried replicating the deep-matching by saying qualType(hasType(hasUnqualifiedDesugaredType(hasDeclaration(... but this doesn't work because hasDeclaration only returns a matcher for a specific type from a subset of subclasses of Type - this is incompatible with expectations of the proposed hasUnqualifiedDesugaredType which takes a Matcher<Type>.
FWIW, the following matcher worked for me:
AST_MATCHER_P(Type, hasUnqualifiedDesugaredType, internal::Matcher<QualType>,
InnerMatcher) {
const Type* type = Node.getUnqualifiedDesugaredType();
QualType qualType(type, 0);
return InnerMatcher.matches(qualType, Finder, Builder);
}
TEST(HasDeclaration, DeepTagType) {
std::string input =
"class Foo {};\n"
"using Bar = Foo;\n"
"using Baz = Bar;\n"
"void Function(Baz param) {}\n";
// Matcher for declaration of the Foo class.
auto param_type_decl_matcher = cxxRecordDecl(hasName("Foo"));
auto m1 = qualType(hasDeclaration(decl(param_type_decl_matcher)));
// hasDeclaration / qualType-flavour.
EXPECT_TRUE(matches(input, parmVarDecl(
hasName("param"),
hasType(hasUnqualifiedDesugaredType(m1)))));
}I think the above will work for my tool - thank you for providing the matcher example (for some reason I incorrectly thought that desugaring would only be done one step a time - this made me think that matching at an arbitrary depth will require tricky things to build a recursive matcher out of a single-step matcher).
BTW: Please note that I am fine with just copy&pasting the matcher above directly into my tool - I don't necessarily need this matcher to be part of the generic library of matchers. I don't have high confidence that the matcher here is 1) applicable broadly (maybe) and 2) what to do about the Type vs QualType issue (yes, it says "unqualified" right in the name, but then it means that it cannot be used with hasDeclaration...).
clang-tidy's modernize-use-auto check has a matcher that desugars one step at a time:
https://reviews.llvm.org/diffusion/L/browse/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp;288204$69
| unittests/ASTMatchers/ASTMatchersTraversalTest.cpp | ||
|---|---|---|
| 253 | deep testing suggestionIf we do end up landing the implementation of hasUnqualifiedDesugaredType from this CL, then I think it might be good to tweak the test so that it verifies that the matching here is "deep" (i.e. that the matcher doesn't just strip a single level of sugar). matches("struct A {}; using B = A; using C = B; C c;", ...deep-vs-shallow-vs-everythingInBetweenAt one point, when discussing the old implementation of hasDeclaration (before https://reviews.llvm.org/D27104) we were wondering whether it should match for all the cases below: std::string input = "struct A {}; using B = A; using C = B; using D = C; D d;";
EXPECT_TRUE(matches(input,
varDecl(...something-hasDeclaration-something...(typeAliasDecl(hasName("B"))))));
EXPECT_TRUE(matches(input,
varDecl(...something-hasDeclaration-something...(typeAliasDecl(hasName("C"))))));
EXPECT_TRUE(matches(input,
varDecl(...something-hasDeclaration-something...(typeAliasDecl(hasName("D"))))));This is not something I want or care about myself, but I wanted to check if the above is something to think about. For example - maybe exposing a singleStepDesugarsTo(...) matcher is better (because it can be used to build the hasUnqualifiedDesugaredType). As I said - this is not very important to me (I only care about full desugaring all the way to a tag type), but it might be something to ponder before committing to the current implementation of hasUnqualifiedDesugaredType. | |
hasUnqualifiedDesugaredType(hasDeclaration(
How about using hasUnqualifiedDesugaredType(recordType(hasDeclaration instead?
I added a test for multi-stage desugaring. Generally, we want to model Clang's AST where it makes sense. The AST has a getUnqualifiedDesugaredType, thus the matcher as it is is expected. We can add single step desugaring or hasAnyDeclaration or similar things when they are actually needed.
This question from my previous comment still stands:
How about using hasUnqualifiedDesugaredType(recordType(hasDeclaration instead?
This looks good to me. We can add finer-grained desugaring later if it's really needed.
| cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 2749 ↗ | (On Diff #79912) | Typo: hasUniqualifeidDesugaredType (actually, two typos). |
deep testing suggestion
If we do end up landing the implementation of hasUnqualifiedDesugaredType from this CL, then I think it might be good to tweak the test so that it verifies that the matching here is "deep" (i.e. that the matcher doesn't just strip a single level of sugar).
matches("struct A {}; using B = A; using C = B; C c;", ...deep-vs-shallow-vs-everythingInBetween
At one point, when discussing the old implementation of hasDeclaration (before https://reviews.llvm.org/D27104) we were wondering whether it should match for all the cases below:
std::string input = "struct A {}; using B = A; using C = B; using D = C; D d;"; EXPECT_TRUE(matches(input, varDecl(...something-hasDeclaration-something...(typeAliasDecl(hasName("B")))))); EXPECT_TRUE(matches(input, varDecl(...something-hasDeclaration-something...(typeAliasDecl(hasName("C")))))); EXPECT_TRUE(matches(input, varDecl(...something-hasDeclaration-something...(typeAliasDecl(hasName("D"))))));This is not something I want or care about myself, but I wanted to check if the above is something to think about. For example - maybe exposing a singleStepDesugarsTo(...) matcher is better (because it can be used to build the hasUnqualifiedDesugaredType).
As I said - this is not very important to me (I only care about full desugaring all the way to a tag type), but it might be something to ponder before committing to the current implementation of hasUnqualifiedDesugaredType.