This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Customize FileCheck prefix in check_clang-tidy.py
ClosedPublic

Authored by zinovy.nis on Apr 18 2018, 9:38 AM.

Details

Summary

The patch introduces a new command line option -check_suffix for check_clang_tidy.py to allow multiple %check_clang_tidy% in tests.

Sample:

// RUN: %check_clang_tidy -check_suffix=-FLAG_1 %s misc-unused-using-decls %t -- -- <option set 1>
// RUN: %check_clang_tidy -check_suffix=-FLAG_2 %s misc-unused-using-decls %t -- -- <option set 2> 
...
+// CHECK-MESSAGES-FLAG_1: :[[@LINE-4]]:10: warning: using decl 'B' is unused [misc-unused-using-decls]
+// CHECK-MESSAGES-FLAG_2: :[[@LINE-7]]:10: warning: using decl 'A' is unused [misc-unused-using-decls]
+// CHECK-FIXES-FLAG_1-NOT: using a::A;$
+// CHECK-FIXES-FLAG_2-NOT: using a::B;$

Diff Detail

Event Timeline

zinovy.nis created this revision.Apr 18 2018, 9:38 AM
zinovy.nis edited the summary of this revision. (Show Details)
zinovy.nis edited the summary of this revision. (Show Details)Apr 18 2018, 9:49 AM

Thank you for the contribution!
Please update the documentation accordingly (http://clang.llvm.org/extra/clang-tidy/#testing-checks).

// RUN: %check_clang_tidy %s misc-unused-using-decls %t -check_suffix=-FLAG_1-- <option set 1>

I don't know whether it makes sense to endorse (or even allow) the use of underscore in the check suffix. The mix of underscores and dashes looks ugly and is prone to errors.

test/clang-tidy/check_clang_tidy.cpp
21

Fix "No newline at end of file"

test/clang-tidy/check_clang_tidy.py
45

Please use dash as a separator as in other arguments.

77

Maybe the script should add a dash when check_suffix is not empty, so that one could use -check-suffix=FLAG instead of -check-suffix=-FLAG?

103

Looks like debug output. Remove?

alexfh requested changes to this revision.Apr 18 2018, 3:08 PM
This revision now requires changes to proceed.Apr 18 2018, 3:08 PM
zinovy.nis marked 4 inline comments as done.
  • Fixed issues pointed by Alexander.
  • Updated docs.
  • Removed exec attribute on file.
zinovy.nis added inline comments.Apr 19 2018, 12:33 AM
test/clang-tidy/check_clang_tidy.py
77

OK.

zinovy.nis edited the summary of this revision. (Show Details)
  • Minor cosmetic fixes.

A few style-related comments.

docs/clang-tidy/index.rst
677

s/SUFFIX_NAME/SUFFIX-NAME/

Same below.

685

I'd use the `-check-suffix=...` format (with the equals sign) for consistency with the documentation.

test/clang-tidy/check_clang_tidy.py
21

`-check-suffix=<file-check-suffix>` and maybe the same for -assume-filename

zinovy.nis marked 3 inline comments as done.
  • Applied the changes suggested by Alexander.
zinovy.nis marked an inline comment as done.Apr 21 2018, 2:15 AM
This revision is now accepted and ready to land.Apr 21 2018, 6:37 AM
This revision was automatically updated to reflect the committed changes.

Did this intentionally omit the possibility to pass a group of prefixes, like FileCheck supports?
Usefulness of this feature is somewhat penalized by this.

Did this intentionally omit the possibility to pass a group of prefixes, like FileCheck supports?
Usefulness of this feature is somewhat penalized by this.

I did not fully realize what do you mean. Can you please provide a sample? I see only single -check-prefix=<string> in FileCheck.

Did this intentionally omit the possibility to pass a group of prefixes, like FileCheck supports?
Usefulness of this feature is somewhat penalized by this.

I did not fully realize what do you mean. Can you please provide a sample? I see only single -check-prefix=<string> in FileCheck.

https://llvm.org/docs/CommandGuide/FileCheck.html#cmdoption-check-prefixes

Did this intentionally omit the possibility to pass a group of prefixes, like FileCheck supports?
Usefulness of this feature is somewhat penalized by this.

I did not fully realize what do you mean. Can you please provide a sample? I see only single -check-prefix=<string> in FileCheck.

https://llvm.org/docs/CommandGuide/FileCheck.html#cmdoption-check-prefixes

Did this intentionally omit the possibility to pass a group of prefixes, like FileCheck supports?
Usefulness of this feature is somewhat penalized by this.

Done. Please review this https://reviews.llvm.org/D52971