[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

[Stephen Concannon via Phabricator via cfe-commits]

Stephen requested review of this revision.
Stephen added a comment.

Thanks for the review! I think I've addressed the comments -- hopefully I kept it inline with what you're thinking. Please have another look at your leisure.



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:82
+  const auto IsSizeTypeOrDifferenceType = hasType(
+      namedDecl(anyOf(hasName("size_type"), hasName("difference_type"))));
+
----------------
aaron.ballman wrote:
> Should this also support `size_t` and `ptrdiff_t` given that those are usually the same as `size_type` and `difference_type`?
Thanks, yeah, that's a good idea. I was somewhat apprehensive to ignore all conversions involving size_t, but it's probably ok. Now that we are, we don't need the ::size() method stuff, so I removed that.

And it seemed like I might as well make the list of types configurable, so I rolled that in here too. So it's a bigger change that you maybe had intended, though hopefully not too far off the mark.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-off.cpp:1
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
+// RUN: -config="{CheckOptions: [ \
----------------
hokein wrote:
> instead of having multiple individual test files, I think we can group them into a single file, you can use the flag `-check-suffix=`, like
> 
> ```
> // RUN: %check_clang_tidy -check-suffix=TESTCASE1 ...
> // RUN: %check_clang_tidy -check-suffix=TESTCASE2 ...
> 
> 
> // some warning code for test-case1
> CHECK-MESSAGES-TESTCASE1: ....
> ``` 
Thanks! Besides cutting down the number of files, it really helped clarify the distinction in behavior with the warning on/off.

I coalesced the on/off pairs into a single, but kept the options tested in separate files, as that seemed to be the pattern in place.

Let me know if you'd prefer to squeeze them into fewer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99543/new/

https://reviews.llvm.org/D99543