[PATCH] C99 partial re-initialization behavior (DR-253)

[Yunzhong Gao]

================
Comment at: lib/AST/StmtProfile.cpp:716-724
@@ -715,1 +715,11 @@
 
+// FIXME: how to test these functions?
+void StmtProfiler::VisitDesignatedInitUpdateExpr(
+    const DesignatedInitUpdateExpr *S) {
+  llvm_unreachable("Not implemented");
+}
+
+void StmtProfiler::VisitNoInitExpr(const NoInitExpr *S) {
+  llvm_unreachable("Not implemented");
+}
+
----------------
rsmith wrote:
> To test these, put a `DesignatedInitUpdateExpr` in the signature of a function template, and declare it twice:
> 
>     template<typename T> void f(decltype(T((char[4]){"foo", [2] = 'x'}))) {} // expected-note {{previous}}
>     template<typename T> void f(decltype(T((char[4]){"foo", [2] = 'r'}))) {} // ok
>     template<typename T> void f(decltype(T((char[4]){"boo", [2] = 'x'}))) {} // ok
>     template<typename T> void f(decltype(T((char[4]){"foo", [2] = 'x'}))) {} // expected-error {{redefinition}}
Looks like StmtProfiler::VisitInitListExpr() only works on the syntactic form, and the syntac form
does not contain any DesignatedInitUpdateExpr. I can leave these two methods unimplemented.
I will add your test cases.

================
Comment at: lib/CodeGen/CGExprCXX.cpp:952-953
@@ -952,3 +952,3 @@
     if (ILE->getNumInits() == 0 && TryMemsetInitialization())
       return;
 
----------------
There is a problem here. If I make a clean checkout newer than r234097 (I used r236298) and
comment out these three lines above, clang would crash with the test case below, even though
they are supposed to be just an optimization. r234095 is the last revision which works without
this assertion.

$ clang -S -std=c++11 -emit-llvm -o - test.cpp
Assertion `!PointeeType || PointeeType == getSourceElementType()' failed.
```
// test.cpp
int n;
struct T { int a; };
void *r = new T[n][3]{ { 1, 2, 3 }, { 4, 5, 6 } };
```

I am not sure if this is a real bug since I had to modify the compiler source, but If I make the rest of
the changes in this patch but leave out the new optimization below, I would be introducing the
same assertion. Maybe I can add this optimization first before the rest of the patch.

================
Comment at: lib/Sema/SemaInit.cpp:2187-2204
@@ +2186,20 @@
+        StructuredList = Result;
+      else if (ExistingInit->isConstantInitializer(SemaRef.Context, false)) {
+        InitListExpr *Result = new (SemaRef.Context) InitListExpr(
+            SemaRef.Context, D->getLocStart(), None, DIE->getLocEnd());
+
+        QualType ResultType = CurrentObjectType;
+        if (!ResultType->isArrayType())
+          ResultType = ResultType.getNonLValueExprType(SemaRef.Context);
+        Result->setType(ResultType);
+
+        // If we decomposed an aggregate initializer into individual components,
+        // we do not issue diagnostics here; instead we wait till later and
+        // issue diagnostics on each individual component.
+        partialMergeFrom(SemaRef.Context, Result, ExistingInit);
+        ExistingInit = nullptr;
+        StructuredList->updateInit(SemaRef.Context, StructuredIndex, Result);
+        StructuredList = Result;
+      }
+      else {
+        if (DesignatedInitUpdateExpr *E =
----------------
rsmith wrote:
> I would prefer to see this case handled by teaching AST/ExprConstant to evaluate `DesignatedInitUpdateExpr` and always using the code below, rather than trying to convert an existing initializer into an `InitListExpr`.
You probably meant lib/CodeGen/CGExprConstant.cpp? I can move the logic of
partialMergeFrom() from Sema into CodeGen. The only down side seems to be that the
diagnostic becomes (arguably) less accurate. To illustrate, say we have the following two cases.

```
struct S {
  char L[6];
};

struct S
Case1[] = {
  "foo",          // expected-note{{previous initialization is here}}
  [0].L[4] = 'x'  // expected-warning{{subobject initialization overrides initialization of other fields}}
};

struct S
Case2[] = {
  { { 'f', 'o', 'o' } },
  [0].L[4] = 'x' // no-warning
};
```
If I break down a string literal into a tree of char literal initializers during semantic analysis, the compiler will treat the two cases the same and issue no warning for either. If I do not break down the string literal and delay till code gen, then in the Sema phase, the compiler only
sees that Case1[0].L in its entirety was initialized with a string literal, and so it will warn that Case1[0].L[4] is being reinitialized. It's a minor issue in my
opinion, but thought I should point it out during review.

http://reviews.llvm.org/D5789

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/