How to verify clang-tidy checks applied to headers?

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
Hi,

I'm trying to write a test for this bug:
<https://bugs.llvm.org/show_bug.cgi?id=26332>

Namely, that clang-tidy check readability-simplify-boolean-expr
doesn't apply to the user's header files correctly.

So I wrote a cpp file like this:

====
// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t

#include "readability-simplify-bool-expr.h"
====

and I copied readability-simplify-bool-expr.cpp to
readability-simplify-bool-expr.h to cover all the same cases in a
header as are covered in a source file.

However, I encounter two problems.  First, I encounter the complaint
from %check_clang_tidy that my source file contains no CHECK lines.
If I add a bogus CHECK-MESSAGES comment to my source file, then I can
continue, but the check still fails to report messages from the
included header.

I tried changing the run line to invoke clang-tidy directly so I could
specify -header-filter, but that also failed to work.

I can run clang-tidy manually with -header-filter and get the
necessary diagnostics, but I need to run this under the test framework
to verify that the diagnostic described in the bug is missing.

How can I achieve this?

Thanks.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
             The Terminals Wiki <http://terminals-wiki.org>
     The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
  Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
Maybe you could first generate the preprocessed file, use the
preprocessed file as the test input and add the "CHECK-MESSAGES" lines
to the preprocessed file?

On Wed, Dec 26, 2018 at 4:39 PM Richard via cfe-dev
<[hidden email]> wrote:

>
> Hi,
>
> I'm trying to write a test for this bug:
> <https://bugs.llvm.org/show_bug.cgi?id=26332>
>
> Namely, that clang-tidy check readability-simplify-boolean-expr
> doesn't apply to the user's header files correctly.
>
> So I wrote a cpp file like this:
>
> ====
> // RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
>
> #include "readability-simplify-bool-expr.h"
> ====
>
> and I copied readability-simplify-bool-expr.cpp to
> readability-simplify-bool-expr.h to cover all the same cases in a
> header as are covered in a source file.
>
> However, I encounter two problems.  First, I encounter the complaint
> from %check_clang_tidy that my source file contains no CHECK lines.
> If I add a bogus CHECK-MESSAGES comment to my source file, then I can
> continue, but the check still fails to report messages from the
> included header.
>
> I tried changing the run line to invoke clang-tidy directly so I could
> specify -header-filter, but that also failed to work.
>
> I can run clang-tidy manually with -header-filter and get the
> necessary diagnostics, but I need to run this under the test framework
> to verify that the diagnostic described in the bug is missing.
>
> How can I achieve this?
>
> Thanks.
> --
> "The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
>              The Terminals Wiki <http://terminals-wiki.org>
>      The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
>   Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
In reply to this post by Richard Smith via cfe-dev
On 27/12/2018 00:39, Richard via cfe-dev wrote:

> Hi,
>
> I'm trying to write a test for this bug:
> <https://bugs.llvm.org/show_bug.cgi?id=26332>
>
> Namely, that clang-tidy check readability-simplify-boolean-expr
> doesn't apply to the user's header files correctly.
>
> So I wrote a cpp file like this:
>
> ====
> // RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
>
> #include "readability-simplify-bool-expr.h"
> ====
>
> and I copied readability-simplify-bool-expr.cpp to
> readability-simplify-bool-expr.h to cover all the same cases in a
> header as are covered in a source file.
>
> However, I encounter two problems.  First, I encounter the complaint
> from %check_clang_tidy that my source file contains no CHECK lines.
> If I add a bogus CHECK-MESSAGES comment to my source file, then I can
> continue, but the check still fails to report messages from the
> included header.
>
> I tried changing the run line to invoke clang-tidy directly so I could
> specify -header-filter, but that also failed to work.
>
> I can run clang-tidy manually with -header-filter and get the
> necessary diagnostics, but I need to run this under the test framework
> to verify that the diagnostic described in the bug is missing.
>
> How can I achieve this?

You probably need to extend the test framework to accept
the -header-filter command line and pass it through.

Thanks,

Stephen.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
[Please reply *only* to the list and do not include my email directly
in the To: or Cc: of your reply; otherwise I will not see your reply.
Thanks.]

In article <q02pmi$6qf$[hidden email]>,
    Stephen Kelly via cfe-dev <[hidden email]> writes:

> You probably need to extend the test framework to accept
> the -header-filter command line and pass it through.

Yeah, I think check_clang_tidy.py will need to be augmented in order
to support that.  However, that's just a wrapper around running
clang-tidy in a convenient manner, so I thought I could prototype it
with // RUN: clang-tidy ..., but didn't manage to find the right
incantation.  I find it hard to believe I'm the first person to try
and do this, so I was hoping there was an accepted recipe for doing
this.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
            The Terminals Wiki <http://terminals-wiki.org>
     The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
  Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
In reply to this post by Richard Smith via cfe-dev
[Just hit 'reply', not 'reply all', please.]

In article <CAJ0ZJHTd=4RwyU3Nh+wNfETYO7D=[hidden email]>,
    Hongbin Zheng <[hidden email]> writes:

> Maybe you could first generate the preprocessed file, use the
> preprocessed file as the test input and add the "CHECK-MESSAGES" lines
> to the preprocessed file?

If I do that, then I'm not testing the actual mechanism.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
            The Terminals Wiki <http://terminals-wiki.org>
     The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
  Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
In reply to this post by Richard Smith via cfe-dev
I don't know much about clang-tidy tests, but i had a quick look.

Because clang-tidy checks are FileCheck-based, it should be trivial to
specify in the main file that the warning needs to come from another
file, from a specific line, by simply matching the output. Matching that
it *doesn't* appear in a certain file on a certain line is more fragile
though, because the other file may change and you won't notice how your
test breaks.

So, yeah, i guess it's better to put CHECK: lines into the header
itself, and then tell FileCheck to use the *header* as the input file.

I'm not sure if %check_clang_tidy aka check_clang_tidy.py supports such
behavior. If not, you should be able to do that manually, but of course
you'll also need to pipe it into FileCheck manually for any checks to
happen.


On 12/26/18 4:39 PM, Richard via cfe-dev wrote:

> Hi,
>
> I'm trying to write a test for this bug:
> <https://bugs.llvm.org/show_bug.cgi?id=26332>
>
> Namely, that clang-tidy check readability-simplify-boolean-expr
> doesn't apply to the user's header files correctly.
>
> So I wrote a cpp file like this:
>
> ====
> // RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
>
> #include "readability-simplify-bool-expr.h"
> ====
>
> and I copied readability-simplify-bool-expr.cpp to
> readability-simplify-bool-expr.h to cover all the same cases in a
> header as are covered in a source file.
>
> However, I encounter two problems.  First, I encounter the complaint
> from %check_clang_tidy that my source file contains no CHECK lines.
> If I add a bogus CHECK-MESSAGES comment to my source file, then I can
> continue, but the check still fails to report messages from the
> included header.
>
> I tried changing the run line to invoke clang-tidy directly so I could
> specify -header-filter, but that also failed to work.
>
> I can run clang-tidy manually with -header-filter and get the
> necessary diagnostics, but I need to run this under the test framework
> to verify that the diagnostic described in the bug is missing.
>
> How can I achieve this?
>
> Thanks.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
In reply to this post by Richard Smith via cfe-dev
Hi,

did you try something similar to `// RUN: clang-tidy -checks='-*,
abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 |
FileCheck %s`?

This pattern occurs multiple times in the test-framework: `git grep "|
FileCheck" *.cpp` shows quite a view places, maybe there are other
places that are more specific to your requirement?


Best, Jonas

Am 27.12.18 um 01:39 schrieb Richard via cfe-dev:

> Hi,
>
> I'm trying to write a test for this bug:
> <https://bugs.llvm.org/show_bug.cgi?id=26332>
>
> Namely, that clang-tidy check readability-simplify-boolean-expr
> doesn't apply to the user's header files correctly.
>
> So I wrote a cpp file like this:
>
> ====
> // RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
>
> #include "readability-simplify-bool-expr.h"
> ====
>
> and I copied readability-simplify-bool-expr.cpp to
> readability-simplify-bool-expr.h to cover all the same cases in a
> header as are covered in a source file.
>
> However, I encounter two problems.  First, I encounter the complaint
> from %check_clang_tidy that my source file contains no CHECK lines.
> If I add a bogus CHECK-MESSAGES comment to my source file, then I can
> continue, but the check still fails to report messages from the
> included header.
>
> I tried changing the run line to invoke clang-tidy directly so I could
> specify -header-filter, but that also failed to work.
>
> I can run clang-tidy manually with -header-filter and get the
> necessary diagnostics, but I need to run this under the test framework
> to verify that the diagnostic described in the bug is missing.
>
> How can I achieve this?
>
> Thanks.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
[Please reply *only* to the list and do not include my email directly
in the To: or Cc: of your reply; otherwise I will not see your reply.

In other words, use 'Reply' not 'Reply All'.

Thanks.]

In article <[hidden email]>,
    Jonas Toth via cfe-dev <[hidden email]> writes:

> did you try something similar to `// RUN: clang-tidy -checks='-*,
> abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 |
> FileCheck %s`?

Yeah, I tried something like that, but I didn't get it to work
properly.  I will look at it again to see if I can get the right
command-line.  It's been a couple years since I worked on clang-tidy
and some of my clang-fu has decayed away.  If I can find a pattern that
works, I'll enhance the helper script to support this directly.

> This pattern occurs multiple times in the test-framework: `git grep "|
> FileCheck" *.cpp` shows quite a view places, maybe there are other
> places that are more specific to your requirement?

Thanks for the pointer.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
            The Terminals Wiki <http://terminals-wiki.org>
     The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
  Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: How to verify clang-tidy checks applied to headers?

Richard Smith via cfe-dev
In reply to this post by Richard Smith via cfe-dev
[Please reply *only* to the list and do not include my email directly
in the To: or Cc: of your reply; otherwise I will not see your reply.

Thanks.]

In article <[hidden email]>,
    Jonas Toth via cfe-dev <[hidden email]> writes:

> did you try something similar to `// RUN: clang-tidy -checks='-*,
> abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 |
> FileCheck %s`?
>
> This pattern occurs multiple times in the test-framework: `git grep "|
> FileCheck" *.cpp` shows quite a view places, maybe there are other
> places that are more specific to your requirement?

Well, it seems that I already had a patch that did this from 2 years
ago that never made it through the exhausting process that is
phabricator.

Right now I have a Compose Method refactor in phabricator that needs
approval.  After that, I'll rebase my old changes on top of that.

So I need a little review love in phabricator, please...

--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
            The Terminals Wiki <http://terminals-wiki.org>
     The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
  Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev