thr3ads.net - llvm dev - [llvm-dev] Creating a clang-tidy const position check [Sep 2016]

If this information is useful, please help other people find it:
Share via:

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

head link

[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

head link

[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

head link

[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

head link

[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

Seemingly Similar Threads

Search for more possibly parallel threads

llvm dev - Sep 2016 - Creating a clang-tidy const position check

[llvm-dev] Creating a clang-tidy const position check

[llvm-dev] Creating a clang-tidy const position check

[llvm-dev] Creating a clang-tidy const position check

[llvm-dev] Creating a clang-tidy const position check

[llvm-dev] Creating a clang-tidy const position check

Seemingly Similar Threads