[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-29 Thread Balázs Benics via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa8120a771143: [clang-tidy] Ignore narrowing conversions in case of bitfields (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D114105?vs=389711=390275#toc Repository: rG

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-25 Thread Clement Courbet via Phabricator via cfe-commits
courbet accepted this revision. courbet added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp:62 +void take(T); +void test_parameter_passing(NoBitfield x) { + take(x.id); [nit]

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-25 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp:1 +// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \ +// RUN: -std=c++17 -- -target x86_64-unknown-linux

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-25 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 389711. steakhal marked an inline comment as done. steakhal added a comment. Added the `test_parameter_passing()` tests, demonstrating the implicit conversion triggered by parameter passing. It turns out my previous revision suppressed a useful report. Now,

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-25 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment. Yes, I think the new approach is what we want. The comments also make it much clearer. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp:1 +// RUN: %check_clang_tidy %s

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. I think it's in pretty good shape, please have a look to help me move this forward. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114105/new/ https://reviews.llvm.org/D114105 ___ cfe-commits mailing list

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 389496. steakhal edited the summary of this revision. steakhal added a comment. Bitshifts have nothing to do with this issue, thus I'm simply matching for bitfield -> `int` implicit casts due to promotions. CHANGES SINCE LAST ACTION

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp:62 +void test(NoBitfield x) { + // no-warning in this function, except for operator+. + static_assert(is_same_v);

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:105 + has(memberExpr(hasDeclaration(fieldDecl(isBitField(, + hasParent( + binaryOperator(anyOf(hasOperatorName("<<"),

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 388240. steakhal marked 2 inline comments as done. steakhal edited the summary of this revision. steakhal added a comment. - added more tests - add a detailed comment about the situation we suppress - see through parenExprs CHANGES SINCE LAST ACTION

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment. Thanks for the details, this explains the motivation well. I think the key point is the combination of: > It turns out doing anything useful with a bitfield will eventually result in > an integral promotion, which mandates that the type must be 'int' if it fits >

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Thanks for the valuable feedback. > D114105#inline-1089282 Let me clarify my motivation, and why I'm interested in bitfields, conversions, and shift expressions. Bitfields can be quite tricky. It turns out doing

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Whisperity via Phabricator via cfe-commits
whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:87 + // Reading a small unsigned bitfield member will be wrapped by an implicit + // cast to 'int' triggering this checker. But this is known to be safe

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:104 + castExpr(hasCastKind(CK_LValueToRValue), + has(memberExpr(hasDeclaration(fieldDecl(isBitField(, + hasParent(

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

2021-11-17 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision. steakhal added reviewers: aaron.ballman, whisperity, alexfh, hokein, JonasToth, courbet, ymandel. Herald added subscribers: carlosgalvezp, martong, rnkovacs, kbarton, kristof.beyls, xazax.hun, nemanjai. steakhal requested review of this revision. Herald added a