[PATCH] D108370: [clang-tidy] Fix wrong FixIt about union in cppcoreguidelines-pro-type-member-init
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 22 11:59:31 PDT 2021
aaron.ballman added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:484
+ AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
if (!FieldsToInit.count(F))
return;
----------------
Given that we're touching this code, we might as well make clang-format happy with it (though I can't quite spot what it wants changed, so if it turns out to be a bad idea for some reason, I don't insist).
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:526
+ union {
+ int K;
+ };
----------------
aaron.ballman wrote:
> Can you add a `CHECK-FIXES-NOT` that we're not adding the fix for this case?
I think you missed this request? Same for the one immediately below.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:534
+
+union U2 {
+ U2() {}
----------------
Sockke wrote:
> aaron.ballman wrote:
> > A related interesting test would be:
> > ```
> > union U3 {
> > U3() {}
> >
> > struct {
> > int B;
> > } b;
> > int A;
> > };
> > ```
> > I would expect `A` or `b` to need initialization for the union, and `B` to need initialization for the struct.
> > A related interesting test would be:
> > ```
> > union U3 {
> > U3() {}
> >
> > struct {
> > int B;
> > } b;
> > int A;
> > };
> > ```
> > I would expect `A` or `b` to need initialization for the union, and `B` to need initialization for the struct.
>
> I apologize for the delay in responding. It seems to me that `struct {}b` has no user-defined constructor, so all members are initialized to their defaults, it is unnecessary to initialize `B` explicitly.
Okay, fair point!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108370/new/
https://reviews.llvm.org/D108370
More information about the cfe-commits
mailing list