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

[clang] Fix unexpected warnings after a01307a #75591

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

Fznamznon
Copy link
Contributor

a01307a broke silencing of -Wmissing-field-initializers warnings in C for nested designators. This fixes the issue.

a01307a broke silencing of -Wmissing-field-initializers warnings in C for
nested designators. This fixes the issue.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 15, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 15, 2023

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

a01307a broke silencing of -Wmissing-field-initializers warnings in C for nested designators. This fixes the issue.


Full diff: https://github.com/llvm/llvm-project/pull/75591.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaInit.cpp (+8)
  • (modified) clang/test/Sema/missing-field-initializers.c (+23)
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index de0d92edb550dd..1cd2198503abaf 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -864,6 +864,14 @@ InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
       WarnIfMissingField &=
           SemaRef.getLangOpts().CPlusPlus || !hasAnyDesignatedInits(SForm);
 
+      if (OuterILE) {
+        InitListExpr *OuterSForm = OuterILE->isSyntacticForm()
+                                       ? OuterILE
+                                       : OuterILE->getSyntacticForm();
+        WarnIfMissingField &= SemaRef.getLangOpts().CPlusPlus ||
+                              !hasAnyDesignatedInits(OuterSForm);
+      }
+
       unsigned NumElems = numStructUnionElements(ILE->getType());
       if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
         ++NumElems;
diff --git a/clang/test/Sema/missing-field-initializers.c b/clang/test/Sema/missing-field-initializers.c
index 8653591ff1187a..8dc8288ad92e6c 100644
--- a/clang/test/Sema/missing-field-initializers.c
+++ b/clang/test/Sema/missing-field-initializers.c
@@ -61,3 +61,26 @@ struct S {
 // f1, now we no longer issue that warning (note, this code is still unsafe
 // because of the buffer overrun).
 struct S s = {1, {1, 2}};
+
+struct S1 {
+  long int l;
+  struct  { int a, b; } d1;
+};
+
+struct S1 s01 = { 1, {1} }; // expected-warning {{missing field 'b' initializer}}
+struct S1 s02 = { .d1.a = 1 }; // designator avoids MFI warning
+
+union U1 {
+  long int l;
+  struct  { int a, b; } d1;
+};
+
+union U1 u01 = { 1 };
+union U1 u02 = { .d1.a = 1 }; // designator avoids MFI warning
+
+struct S2 {
+  long int l;
+  struct { int a, b; struct {int c; } d2; } d1;
+};
+
+struct S2 s22 = { .d1.d2.c = 1 }; // designator avoids MFI warning

Copy link
Collaborator

@mikaelholmen mikaelholmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't know this, and I have no easy way to test that this really fixes all the new warnings we see with a01307a, but at least it seems to fix the added cases in the missing-field-initializers.c test so it looks like a step in the right direction.

clang/lib/Sema/SemaInit.cpp Show resolved Hide resolved
Copy link
Collaborator

@mikaelholmen mikaelholmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, ideally this should be reviewed by someone who knows the code but it LGTM.

@Fznamznon
Copy link
Contributor Author

Ok, ideally this should be reviewed by someone who knows the code but it LGTM.

I know, but a lot of folks are now on vacation already. The change is not complicated, so I'll probably wait till green pre-commit, then leave it for post-commit review to resolve the issue.

@mikaelholmen
Copy link
Collaborator

Ok, ideally this should be reviewed by someone who knows the code but it LGTM.

I know, but a lot of folks are now on vacation already. The change is not complicated, so I'll probably wait till green pre-commit, then leave it for post-commit review to resolve the issue.

Yeah, sounds good. Thanks!

@Fznamznon
Copy link
Contributor Author

Ok, adding more reviewers. Please feel free to do post-commit review.

@Fznamznon Fznamznon merged commit 32d5221 into llvm:main Dec 15, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants