Page MenuHomePhabricator

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

Authored by al3xtjames on May 12 2019, 2:02 PM.

Details

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.

Diff Detail

Repository
rC Clang

Event Timeline

al3xtjames created this revision.May 12 2019, 2:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2019, 2:02 PM
al3xtjames edited the summary of this revision. (Show Details)

Formatting changes.

al3xtjames edited the summary of this revision. (Show Details)May 12 2019, 4:28 PM

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'

Such nested structures work fine (no warnings generated).

Lekensteyn accepted this revision.May 16 2019, 5:25 PM

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

This revision is now accepted and ready to land.May 16 2019, 5:25 PM

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

rsmith added a subscriber: rsmith.May 18 2019, 3:55 PM
rsmith added inline comments.
clang/lib/AST/Expr.cpp
2178–2179

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

const IntegerLiteral *Lit = dyn_cast<IntegerLiteral>(getInit(0)->IgnoreImplicit());
al3xtjames updated this revision to Diff 200159.EditedMay 18 2019, 5:01 PM

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

Lekensteyn accepted this revision.May 20 2019, 10:00 AM

Verified again!

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

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