Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A false positive -Wmissing-field-initializers for anonymous unions #70444

Closed
Fznamznon opened this issue Oct 27, 2023 · 5 comments
Closed

A false positive -Wmissing-field-initializers for anonymous unions #70444

Fznamznon opened this issue Oct 27, 2023 · 5 comments
Assignees
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer confirmed Verified by a second party duplicate Resolved as duplicate false-positive Warning fires when it should not

Comments

@Fznamznon
Copy link
Contributor

https://reviews.llvm.org/D157879 introduced a false positive for anonymous union members:

struct A {
    int m;
    union { int n = 0; };
};

A a = A{.m = 0};

now produces a false positive warning saying that the anonymous union member in A is uninitialized.

Thanks @zygoloid for reporting this.

@Fznamznon Fznamznon added clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party labels Oct 27, 2023
@Fznamznon Fznamznon self-assigned this Oct 27, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 27, 2023

@llvm/issue-subscribers-clang-frontend

Author: Mariya Podchishchaeva (Fznamznon)

https://reviews.llvm.org/D157879 introduced a false positive for anonymous union members:
struct A {
    int m;
    union { int n = 0; };
};

A a = A{.m = 0};

now produces a false positive warning saying that the anonymous union member in A is uninitialized.

Thanks @zygoloid for reporting this.

@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer false-positive Warning fires when it should not and removed clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 27, 2023
@Fznamznon
Copy link
Contributor Author

I gave it some thought.... Actually, should clang warn about it at all?
https://reviews.llvm.org/D157879 only enabled warnings for designators, the problem has been there before, can be reproduced without designators - https://godbolt.org/z/7M5o9vWY4. Note that gcc reports this as well.
If we read "missing initializer" as not explicitly provided in the list then the warning is correct.
If we choose to not report this particular case, should we also not report something like:

union B {
    int a = 1;
    float b;
};

struct A {
    int m;
    B BB;
};
void foo() {
  A a = {.m = 1};
}

https://godbolt.org/z/7ev8Y95Y7

or should we not report something like this:

struct A {
    int m;
    union { int a; union { short b; float cc = 1; }; };

};
void foo() {
  A a = {.m = 1};
}

The latter asserts clang atm though.

cc @AaronBallman @shafik for opinion.

@shafik
Copy link
Collaborator

shafik commented Oct 30, 2023

It seems like there is a lot of debate on this diagnostic: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96868#c3

I can see points on both sides of this discussion. There seems to be sufficient confusion that maybe we should have a separate diagnostic.

@zygoloid
Copy link
Collaborator

If we read "missing initializer" as not explicitly provided in the list then the warning is correct.

Sure, but in general the warning does not warn on members with default member initializers, because in that case the initializer isn't missing -- it's present, in the class definition. In this case, there is a default member initializer.

If we choose to not report this particular case, should we also not report something like:

union B {
    int a = 1;
    float b;
};

struct A {
    int m;
    B BB;
};
void foo() {
  A a = {.m = 1};
}

This situation is different, because the author of struct A did not explicitly say that the initializer of BB is optional. In the original example, they did. They could write B BB = {}; if that's what they mean. In the original example, they can't.

or should we not report something like this:

struct A {
    int m;
    union { int a; union { short b; float cc = 1; }; };

};
void foo() {
  A a = {.m = 1};
}

We shouldn't diagnose that under this warning flag. But we should warn that this is not valid C++ (anonymous unions can't be nested) and perhaps warn that it's pointless to nest them (flattening the two unions into one would change nothing).

@Fznamznon
Copy link
Contributor Author

Duplicate.

@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Oct 31, 2023
@EugeneZelenko EugeneZelenko added the duplicate Resolved as duplicate label Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer confirmed Verified by a second party duplicate Resolved as duplicate false-positive Warning fires when it should not
Projects
None yet
Development

No branches or pull requests

5 participants