Matthias Vallentin via llvm-dev
2016-Sep-21 19:48 UTC
[llvm-dev] Creating a clang-tidy const position check
I'm in the process of writing a clang-tidy check that concerns the position of the "const" keyword. The check should either enforce "T const" or "const T", based on a flag. Naturally, this should extend to all sorts of variations and qualifiers, e.g., X<const T&, Y const*> const&. My approach is to first find the right AST matcher expression to flag style violations, and then use the lexer (getLocStart/End) to correct them [1]. Does that sound reasonable? There's also Qualifiers::removeConst and Qualifiers::addConst, but I'm not sure how it affects the position of const. I have trouble with finding all const-qualified types. I looked at misc-misplaced-const for inspiration, which contains this matcher: valueDecl(hasType(isConstQualified())) Why doesn't this yield a match? My test code has this form: template <class T> struct type {}; template <class T> void f(const type<const double>&, const T&) { } using alias = const int&; I've tested this expression with clang-query (master branch). Any help that steers me in the right direction would be much appreciated. Matthias
Piotr Padlewski via llvm-dev
2016-Sep-21 21:09 UTC
[llvm-dev] Creating a clang-tidy const position check
2016-09-21 12:48 GMT-07:00 Matthias Vallentin via llvm-dev < llvm-dev at lists.llvm.org>:> I'm in the process of writing a clang-tidy check that concerns the > position of the "const" keyword. The check should either enforce "T > const" or "const T", based on a flag. Naturally, this should extend to > all sorts of variations and qualifiers, e.g., X<const T&, Y const*> > const&. > > My approach is to first find the right AST matcher expression to flag > style violations, and then use the lexer (getLocStart/End) to correct > them [1]. Does that sound reasonable? There's also > Qualifiers::removeConst and Qualifiers::addConst, but I'm not sure how > it affects the position of const. > > Writing simple matcher and then do lexer magic seems the best idea.> I have trouble with finding all const-qualified types. I looked at > misc-misplaced-const for inspiration, which contains this matcher: > > valueDecl(hasType(isConstQualified())) > > Why doesn't this yield a match? My test code has this form: > > template <class T> > struct type {}; > > template <class T> > void f(const type<const double>&, const T&) { > } > > Not really sure, but I would guess that this matcher might work only fortemplate instantiations (if you will use this function somewhere)[ I guess the misc-misplaced-const might have a bug.> using alias = const int&; >I guess this doesn't match because it is alias declaration. Use typedefNameDecl> > I've tested this expression with clang-query (master branch). Any help > that steers me in the right direction would be much appreciated. > > Matthias >Piotr _______________________________________________> LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160921/660de7da/attachment.html>
Aaron Ballman via llvm-dev
2016-Sep-21 21:30 UTC
[llvm-dev] Creating a clang-tidy const position check
On Wed, Sep 21, 2016 at 3:48 PM, Matthias Vallentin via llvm-dev <llvm-dev at lists.llvm.org> wrote:> I'm in the process of writing a clang-tidy check that concerns the > position of the "const" keyword. The check should either enforce "T > const" or "const T", based on a flag. Naturally, this should extend to > all sorts of variations and qualifiers, e.g., X<const T&, Y const*> > const&. > > My approach is to first find the right AST matcher expression to flag > style violations, and then use the lexer (getLocStart/End) to correct > them [1]. Does that sound reasonable? There's also > Qualifiers::removeConst and Qualifiers::addConst, but I'm not sure how > it affects the position of const. > > I have trouble with finding all const-qualified types. I looked at > misc-misplaced-const for inspiration, which contains this matcher: > > valueDecl(hasType(isConstQualified())) > > Why doesn't this yield a match? My test code has this form: > > template <class T> > struct type {}; > > template <class T> > void f(const type<const double>&, const T&) { > } > > using alias = const int&; > > I've tested this expression with clang-query (master branch). Any help > that steers me in the right direction would be much appreciated.In all three cases, you do not have a const qualified type, you have a reference to a const qualified type. You could try something like valueDecl(hasType(references(qualType(isConstQualified())))). Note that the type alias will not be matched regardless because it is not a ValueDecl. ~Aaron> > Matthias > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Matthias Vallentin via llvm-dev
2016-Sep-22 00:17 UTC
[llvm-dev] Creating a clang-tidy const position check
Thanks for your help, Piotr and Aaraon. I got further with my match expression, which now looks like this: void ConstPositionCheck::registerMatchers(MatchFinder *Finder) { auto const_qualified = hasType( anyOf( references(qualType(isConstQualified())), pointsTo(qualType(isConstQualified())), qualType(isConstQualified()))); Finder->addMatcher(valueDecl(const_qualified).bind("value"), this); Finder->addMatcher(typedefDecl(const_qualified).bind("typedef"), this); Finder->addMatcher(typeAliasDecl(const_qualified).bind("alias"), this); Finder->addMatcher(templateTypeParmDecl().bind("template"), this); Finder->addMatcher(nonTypeTemplateParmDecl().bind("nontemplate"), this); } The idea is to catch references, pointers, and raw types with a const qualification. For the templateTypeParmDecl(), I'll be looking at the type obtained via getDefaultArgument. Similarly, for nonTypeTemplateParmDecl I'll inspect the type itself. It seems to work in clang-query, i.e., if I type let cq hasType(anyOf(...)) and then play with match valueDecl(cq), I get the results I want. However, the above C++ code doesn't compile yet: ../extra/clang-tidy/misc/ConstPositionCheck.cpp:23:26: error: call to 'hasType' is ambiguous auto const_qualified = hasType( ^~~~~~~ I tried to debug this error but the macro magic discouraged me from diving deeper. What's the correct way to share the hasType(..) match expression between matchers? Also, if you can think of any other match expressions that I'm forgetting, please let me know. Matthias
Aaron Ballman via llvm-dev
2016-Sep-22 14:21 UTC
[llvm-dev] Creating a clang-tidy const position check
On Wed, Sep 21, 2016 at 8:17 PM, Matthias Vallentin via llvm-dev <llvm-dev at lists.llvm.org> wrote:> Thanks for your help, Piotr and Aaraon. I got further with my match > expression, which now looks like this: > > void ConstPositionCheck::registerMatchers(MatchFinder *Finder) { > auto const_qualified = hasType( > anyOf( > references(qualType(isConstQualified())), > pointsTo(qualType(isConstQualified())), > qualType(isConstQualified()))); > Finder->addMatcher(valueDecl(const_qualified).bind("value"), this); > Finder->addMatcher(typedefDecl(const_qualified).bind("typedef"), this); > Finder->addMatcher(typeAliasDecl(const_qualified).bind("alias"), this); > Finder->addMatcher(templateTypeParmDecl().bind("template"), this); > Finder->addMatcher(nonTypeTemplateParmDecl().bind("nontemplate"), this); > } > > The idea is to catch references, pointers, and raw types with a const > qualification. For the templateTypeParmDecl(), I'll be looking at the > type obtained via getDefaultArgument. Similarly, for > nonTypeTemplateParmDecl I'll inspect the type itself. It seems to work > in clang-query, i.e., if I type > > let cq hasType(anyOf(...)) > > and then play with match valueDecl(cq), I get the results I want. > However, the above C++ code doesn't compile yet: > > ../extra/clang-tidy/misc/ConstPositionCheck.cpp:23:26: error: call to 'hasType' is ambiguous > auto const_qualified = hasType( > ^~~~~~~ > > I tried to debug this error but the macro magic discouraged me from > diving deeper. What's the correct way to share the hasType(..) match > expression between matchers? > > Also, if you can think of any other match expressions that I'm > forgetting, please let me know.The C++ DSL is more picky about type information than the clang-query dynamic matchers. The problem here is that anyOf() is not return a specific enough type for hasType() to handle, so hasType() is an ambiguous call. You should be able to do something like this instead: hasType(qualType(anyOf(...))). ~Aaron> > Matthias > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev