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

[Richard Smith]

================
Comment at: lib/AST/Expr.cpp:2779
@@ +2778,3 @@
+      return false;
+    assert(false && "DIUE->getBase() should not be a constant expression");
+    break;
----------------
ygao wrote:
> 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?
I don't think we should be calling this during initialization at all. If we really want to pull apart, say, a StringLiteral into an InitListExpr of CharacterLiterals (and I'm not convinced that we do, for source fidelity reasons), we should just do that directly in SemaInit without regard to whether the initializer supports constant emission.

================
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();
+}
+
----------------
ygao wrote:
> 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.
OK, good point. Nonetheless, the range from `getLocStart()` to `getLocEnd()` should cover a contiguous range of code covering only this expression, and if the brace locations are invalid, then we'll use `Base->getLocStart()` to `Updater->getLocEnd()`, which might cover other stuff.

http://reviews.llvm.org/D5789

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