[Sema] Suppress additional warnings for C's zero initializer

[al3xtjames]

Summary

D28148 relaxed some checks for assigning { 0 } to a structure for all C standards, but it failed to handle structures with non-integer subobjects. Relax -Wmissing-braces checks for such structures, and add some additional tests.

This fixes PR39931.

[al3xtjames]

Formatting changes.

[Lekensteyn]

This looks reasonable to fix the problem at hand, but would it handle nested structures too?

struct s1 {
   short f1;  // "int f1" is fine.
};
struct s2 {
   struct s1 f2;
   int x;
};
struct s3 {
   struct s2 f3;
   int x;
};
struct s2 x1 = {0};
struct s3 x2 = {0};

produces this AST (clang-check -ast-dump x.c --):

|-VarDecl 0x55e7bbc60390 <line:13:1, col:18> col:11 x1 'struct s2':'struct s2' cinit
| `-InitListExpr 0x55e7bbc60498 <col:16, col:18> 'struct s2':'struct s2'
|   |-InitListExpr 0x55e7bbc604e8 <col:17> 'struct s1':'struct s1'
|   | `-ImplicitCastExpr 0x55e7bbc60530 <col:17> 'short' <IntegralCast>
|   |   `-IntegerLiteral 0x55e7bbc60430 <col:17> 'int' 0
|   `-ImplicitValueInitExpr 0x55e7bbc60548 <<invalid sloc>> 'int'
|-VarDecl 0x55e7bbc605b0 <line:14:1, col:18> col:11 x2 'struct s3':'struct s3' cinit
| `-InitListExpr 0x55e7bbc60678 <col:16, col:18> 'struct s3':'struct s3'
|   |-InitListExpr 0x55e7bbc606c8 <col:17> 'struct s2':'struct s2'
|   | |-InitListExpr 0x55e7bbc60718 <col:17> 'struct s1':'struct s1'
|   | | `-ImplicitCastExpr 0x55e7bbc60760 <col:17> 'short' <IntegralCast>
|   | |   `-IntegerLiteral 0x55e7bbc60610 <col:17> 'int' 0
|   | `-ImplicitValueInitExpr 0x55e7bbc60778 <<invalid sloc>> 'int'
|   `-ImplicitValueInitExpr 0x55e7bbc60788 <<invalid sloc>> 'int'

[al3xtjames]

Such nested structures work fine (no warnings generated).

[Lekensteyn]

Thanks, I have also verified this patch against the above test case.

[al3xtjames]

Thanks for reviewing! I don't have commit access, so I can't commit this patch myself.

[rsmith]

clang/lib/AST/Expr.cpp

2092–2100

Use Expr::IgnoreImplicit rather than checking for an ImplicitCastExpr here:

const IntegerLiteral *Lit = dyn_cast<IntegerLiteral>(getInit(0)->IgnoreImplicit());

[al3xtjames]

Switched to using Expr::IgnoreImplicit() instead of manually checking for an ImplicitCastExpr.

[Lekensteyn]

Verified again!

[Lekensteyn]

@rsmith Are you happy with the changes, is it ready to be merged?

[Lekensteyn]

@rsmith The latest patch version has addressed your feedback, can you confirm that this is ready to be merged?

[Lekensteyn]

@al3xtjames I was about to commit this but noticed that some others check whether getInit(0) is NULL or not before proceeding. Should that be done here as well? If not, why?

See for example "Harden InitListExpr::isStringLiteralInit() against getInit() returning null." in
https://github.com/llvm/llvm-project/commit/256bd96d1c2464b0f0ecb482ca52075a3158f6a1#diff-dac09655ff6a54658c320a28a6ea297c
and InitListExpr::isTransparent in
https://github.com/llvm/llvm-project/commit/122f88d481971b68d05ba12395c3b73ab4b31ab3#diff-dac09655ff6a54658c320a28a6ea297c

[al3xtjames]

Added null check for getInit(0)

[Lekensteyn]

Thanks, I'll push once the build and test pass.

[al3xtjames]

Thanks!

clang/lib/AST/Expr.cpp

 
 bool InitListExpr::isIdiomaticZeroInitializer(const LangOptions &LangOpts) const {
   assert(isSyntacticForm() && "only test syntactic form as zero initializer");
 
   if (LangOpts.CPlusPlus || getNumInits() != 1) {
     return false;
   }
 
   const IntegerLiteral *Lit = dyn_cast<IntegerLiteral>(getInit(0));
+  if (!Lit) {
+    const ImplicitCastExpr *Cast = dyn_cast<ImplicitCastExpr>(getInit(0));
+    if (!Cast)
+      return false;
+    Lit = dyn_cast<IntegerLiteral>(Cast->getSubExpr());
+  }
+
   return Lit && Lit->getValue() == 0;
 }
 
 SourceLocation InitListExpr::getBeginLoc() const {
   if (InitListExpr *SyntacticForm = getSyntacticForm())
     return SyntacticForm->getBeginLoc();
   SourceLocation Beg = LBraceLoc;
   if (Beg.isInvalid()) {
     // Find the first non-null initializer.

clang/test/Sema/zero-initializer.c

 // RUN: %clang_cc1 -std=c99 -Wmissing-field-initializers -Wmissing-braces -verify %s
 
 // Tests that using {0} in struct initialization or assignment is supported
 struct foo { int x; int y; };
 struct bar { struct foo a; struct foo b; };
 struct A { int a; };
 struct B { struct A a; };
 struct C { struct B b; };
 struct D { struct C c; int n; };
+struct E { short e; };
+struct F { struct E e; int n; };
 
 int main(void)
 {
   struct foo f = { 0 }; // no-warning
   struct foo g = { 9 }; // expected-warning {{missing field 'y' initializer}}
   struct foo h = { 9, 9 }; // no-warning
   struct bar i = { 0 }; // no-warning
   struct bar j = { 0, 0 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'b' initializer}}
   struct bar k = { { 9, 9 }, { 9, 9 } }; // no-warning
   struct bar l = { { 9, 9 }, { 0 } }; // no-warning
   struct bar m = { { 0 }, { 0 } }; // no-warning
   struct bar n = { { 0 }, { 9, 9 } }; // no-warning
   struct bar o = { { 9 }, { 9, 9 } }; // expected-warning {{missing field 'y' initializer}}
   struct C p = { 0 }; // no-warning
   struct C q = { 9 }; // warning suppressed for struct with single element
   struct D r = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  struct F s = { 0 }; // no-warning
+  struct F t = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+
   f = (struct foo ) { 0 }; // no-warning
   g = (struct foo ) { 9 }; // expected-warning {{missing field 'y' initializer}}
   h = (struct foo ) { 9, 9 }; // no-warning
   i = (struct bar) { 0 }; // no-warning
   j = (struct bar) { 0, 0 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'b' initializer}}
   k = (struct bar) { { 9, 9 }, { 9, 9 } }; // no-warning
   l = (struct bar) { { 9, 9 }, { 0 } }; // no-warning
   m = (struct bar) { { 0 }, { 0 } }; // no-warning
   n = (struct bar) { { 0 }, { 9, 9 } }; // no-warning
   o = (struct bar) { { 9 }, { 9, 9 } }; // expected-warning {{missing field 'y' initializer}}
   p = (struct C) { 0 }; // no-warning
   q = (struct C) { 9 }; // warning suppressed for struct with single element
   r = (struct D) { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
+  s = (struct F) { 0 }; // no-warning
+  t = (struct F) { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'n' initializer}}
 
   return 0;
 }