[Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

[Nathan-Huckleberry]

Summary

Do not automatically report self references of structs in statement expression
as warnings. Instead wait for uninitialized cfg analysis.
https://bugs.llvm.org/show_bug.cgi?id=42604

[Nathan-Huckleberry]

• Adding fix

[Nathan-Huckleberry]

• Fixed style

[nickdesaulniers]

Thanks for the patch. Minor nits looking for test cases that should have no warning (like the two examples provided in the bug).

clang/test/Sema/warn-uninitialized-statement-expression.c

7

Can you please add a case like foo without z in it (so that no warnings are expected, as in pr42604)? (You can keep both test cases, just looking for 1 more, as the first case).

20

Ditto about adding a clear case that should not warn.

21

This needs a trailing comment like:

int x = my_widget.x; // fixme: we should probably warn about this case

and file a bug about it. Doesn't need to be solved here.

[Nathan-Huckleberry]

• Add warning-free test cases

[nickdesaulniers]

clang/lib/Sema/SemaDecl.cpp

10890

Should just be dyn_cast<StmtExpr>(E)?
http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates

clang/test/Sema/warn-uninitialized-statement-expression.c

36

Can you please file a bug pointing to this test case?

[Nathan-Huckleberry]

• Change cast type

[Nathan-Huckleberry]

clang/test/Sema/warn-uninitialized-statement-expression.c

21

https://bugs.llvm.org/show_bug.cgi?id=42625

[rsmith]

I don't think this problem really has anything to do with statement expressions; consider:

struct Widget a = (init2(&a), a);

... which has the same behaviour and presumably produces the same warning.

It's just a C / C++ difference. In C++, these examples are undefined because the lifetime of the object hasn't started yet, but I think in C they're valid. We should just disable the whole warning in C mode and leave it to the CFG analysis.

[Nathan-Huckleberry]

• Disable self reference checking for C

[nickdesaulniers]

Thanks for the patch and following up on the review.

[aaron.ballman]

LGTM aside from a comment request.

clang/lib/Sema/SemaDecl.cpp

11265

This should probably have a comment explaining why we only do it in C++ mode.

[Nathan-Huckleberry]

• Add comment

clang/lib/Sema/SemaDecl.cpp

static void CheckSelfReference(Sema &S, Decl* OrigDecl, Expr *E,
     // Doing so is a way to silence uninitialized warnings.
     if (!DirectInit && !cast<VarDecl>(OrigDecl)->getType()->isRecordType())
       if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
         if (ICE->getCastKind() == CK_LValueToRValue)
           if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(ICE->getSubExpr()))
             if (DRE->getDecl() == OrigDecl)
               return;
 
+    if (cast<VarDecl>(OrigDecl)->getType()->isRecordType() &&
+        dyn_cast_or_null<StmtExpr>(E)) {
+      return;
+    }
+
     SelfReferenceChecker(S, OrigDecl).CheckExpr(E);
   }
 } // end anonymous namespace
 
 namespace {
   // Simple wrapper to add the name of a variable or (if no variable is
   // available) a DeclarationName into a diagnostic.
   struct VarDeclOrName {
if (!VDecl->isInvalidDecl()) {
     Init = Result.getAs<Expr>();
   }
 
   // Check for self-references within variable initializers.
   // Variables declared within a function/method body (except for references)
   // are handled by a dataflow analysis.
   if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() ||
       VDecl->getType()->isReferenceType()) {
     CheckSelfReference(*this, RealDecl, Init, DirectInit);
   }
 
   // If the type changed, it means we had an incomplete type that was
   // completed by the initializer. For example:
   //   int ary[] = { 1, 3, 5 };
   // "ary" transitions from an IncompleteArrayType to a ConstantArrayType.
   if (!VDecl->isInvalidDecl() && (DclT != SavT))
     VDecl->setType(DclT);

clang/test/Sema/warn-uninitialized-statement-expression.c

+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
+void init(int *);
+
+void foo(void) {
+  int i = ({
+    int z = i; // expected-warning{{variable 'i' is uninitialized when used within its own initialization}}
+    init(&i);
+    i;
+  });
+}
+
+struct widget {
+  int x, y;
+};
+void init2(struct widget *);
+
+void bar(void) {
+  struct widget my_widget = ({
+    struct widget z = my_widget; // expected-warning{{variable 'my_widget' is uninitialized when used within its own initialization}}
+    int x = my_widget.x;
+    init2(&my_widget);
+    my_widget;
+  });
+}
+
+void baz(void) {
+  struct widget a = ({
+    struct widget b = ({
+      b = a; // expected-warning{{variable 'a' is uninitialized when used within its own initialization}}
+    });
+    a;
+  });
+}
+
+void f(void) {
+  struct widget *a = ({
+    init2(a); // expected-warning{{variable 'a' is uninitialized when used within its own initialization}}
+    a;
+  });
+}