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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
