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

[Yunzhong Gao]

Thanks for reviewing this.


================
Comment at: lib/AST/Expr.cpp:2779
@@ +2778,3 @@
+      return false;
+    assert(false && "DIUE->getBase() should not be a constant expression");
+    break;
----------------
rsmith wrote:
> I would think this should be:
> 
>     return Base->isConstantInitializer(...) && Update->isConstantInitializer(...)
This function may get called in the middle of the initialization process, at which point the updater still contains unfilled "holes". I do not really want to call isConstantInitialzier() on such an updater, therefore the assertion here. Hmm, since I am so confident that Base will not be a constant initializer, maybe I can even just set culprit to Base without running isConstantInitializer() on it?

================
Comment at: lib/AST/Expr.cpp:4001
@@ +4000,3 @@
+
+  InitListExpr *ILE = new (C) InitListExpr(C, lbraceloc, None, rbraceloc);
+  ILE->setType(baseExpr->getType());
----------------
rsmith wrote:
> Given that we store the brace locations on the Updater, do we need to also store them inside `DesignatedInitUpdateExpr`?
I think you are right. I can get the brace locations from the updater without storing a duplicate copy.

================
Comment at: lib/AST/Expr.cpp:4006-4016
@@ +4005,13 @@
+
+SourceLocation DesignatedInitUpdateExpr::getLocStart() const {
+  if (LBraceLoc.isValid())
+    return LBraceLoc;
+  return getBase()->getLocStart();
+}
+
+SourceLocation DesignatedInitUpdateExpr::getLocEnd() const {
+  if (RBraceLoc.isValid())
+    return RBraceLoc;
+  return getUpdater()->getLocEnd();
+}
+
----------------
rsmith wrote:
> This will give a range of Base start to Updater end if there's no LBraceLoc / RBraceLoc, which is a source range that can contain things unrelated to the update. Updater's end loc is always the same as RBraceLoc anyway, so should this instead return `getBase()->getLocEnd()`?
> Updater's end loc is always the same as RBraceLoc anyway.

Are you sure? If the RBraceLoc is not valid, then the updater's RBraceLoc is not valid either, so updater->getLocEnd() should return the end location of the last non-null initializer.

http://reviews.llvm.org/D5789

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