Reimplement the handling of the "current object" in designator

initializers, so that we are within the appropriate subobject after
we've processed a multi-designator designation. We're matching GCC and
EDG's behavior on all examples I've found thus far.

*Huge* thanks to Eli Friedman for pointing out my fundamental
misunderstanding of "current object" in the C99 spec.

llvm-svn: 62812

clang/include/clang/AST/Expr.h

@@ -1853,6 +1853,11 @@ public:
       Field.NameOrField = reinterpret_cast<uintptr_t>(FD);
     }
 
+    SourceLocation getDotLoc() const {
+      assert(Kind == FieldDesignator && "Only valid on a field designator");
+      return SourceLocation::getFromRawEncoding(Field.DotLoc);
+    }
+
     SourceLocation getFieldLoc() const {
       assert(Kind == FieldDesignator && "Only valid on a field designator");
       return SourceLocation::getFromRawEncoding(Field.FieldLoc);

clang/lib/Sema/Sema.h

@@ -19,6 +19,7 @@
 #include "CXXFieldCollector.h"
 #include "SemaOverload.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/Expr.h"
 #include "clang/Parse/Action.h"
 #include "clang/Basic/Diagnostic.h"
 #include "llvm/ADT/SmallVector.h"
@@ -1838,6 +1839,7 @@ class InitListChecker {
                              unsigned &Index);
 
   void CheckListElementTypes(InitListExpr *IList, QualType &DeclType, 
+                             bool SubobjectIsDesignatorContext, 
                              unsigned &Index);
   void CheckSubElementType(InitListExpr *IList, QualType ElemType, 
                            Expr *expr, unsigned &Index);
@@ -1846,11 +1848,17 @@ class InitListChecker {
                        Expr *expr, unsigned &Index);
   void CheckVectorType(InitListExpr *IList, QualType DeclType, unsigned &Index);
   void CheckStructUnionTypes(InitListExpr *IList, QualType DeclType, 
-                             unsigned &Index);
+                             RecordDecl::field_iterator Field, 
-  void CheckArrayType(InitListExpr *IList, QualType &DeclType, unsigned &Index);
+                             bool SubobjectIsDesignatorContext, unsigned &Index);
+  void CheckArrayType(InitListExpr *IList, QualType &DeclType, 
+                      llvm::APSInt elementIndex, 
+                      bool SubobjectIsDesignatorContext, unsigned &Index);
   bool CheckDesignatedInitializer(InitListExpr *IList, DesignatedInitExpr *DIE, 
-                                  QualType DeclType, FieldDecl *&DesignatedField, 
+                                  DesignatedInitExpr::designators_iterator D,
-                                  llvm::APSInt &DesignatedIndex, unsigned &Index);
+                                  QualType &CurrentObjectType, 
+                                  RecordDecl::field_iterator *NextField,
+                                  llvm::APSInt *NextElementIndex,
+                                  unsigned &Index);
   int numArrayElements(QualType DeclType);
   int numStructUnionElements(QualType DeclType);
 public:

clang/lib/Sema/SemaInit.cpp

@@ -74,7 +74,7 @@ void InitListChecker::CheckImplicitInitList(InitListExpr *ParentIList,
   // Check the element types *before* we create the implicit init list;
   // otherwise, we might end up taking the wrong number of elements
   unsigned NewIndex = Index;
-  CheckListElementTypes(ParentIList, T, NewIndex);
+  CheckListElementTypes(ParentIList, T, false, NewIndex);
 
   for (int i = 0; i < maxElements; ++i) {
     // Don't attempt to go past the end of the init list
@@ -101,7 +101,7 @@ void InitListChecker::CheckExplicitInitList(InitListExpr *IList, QualType &T,
                                             unsigned &Index) {
   assert(IList->isExplicit() && "Illegal Implicit InitListExpr");
 
-  CheckListElementTypes(IList, T, Index);
+  CheckListElementTypes(IList, T, true, Index);
   IList->setType(T);
   if (hadError)
     return;
@@ -130,16 +130,22 @@ void InitListChecker::CheckExplicitInitList(InitListExpr *IList, QualType &T,
 
 void InitListChecker::CheckListElementTypes(InitListExpr *IList,
                                             QualType &DeclType, 
+                                            bool SubobjectIsDesignatorContext,
                                             unsigned &Index) {
   if (DeclType->isScalarType()) {
     CheckScalarType(IList, DeclType, 0, Index);
   } else if (DeclType->isVectorType()) {
     CheckVectorType(IList, DeclType, Index);
   } else if (DeclType->isAggregateType() || DeclType->isUnionType()) {
-    if (DeclType->isStructureType() || DeclType->isUnionType())
+    if (DeclType->isStructureType() || DeclType->isUnionType()) {
-      CheckStructUnionTypes(IList, DeclType, Index);
+      RecordDecl *RD = DeclType->getAsRecordType()->getDecl();
-    else if (DeclType->isArrayType()) 
+      CheckStructUnionTypes(IList, DeclType, RD->field_begin(), 
-      CheckArrayType(IList, DeclType, Index);
+                            SubobjectIsDesignatorContext, Index);
+    } else if (DeclType->isArrayType()) {
+      // FIXME: Is 32 always large enough for array indices?
+      llvm::APSInt Zero(32, false);
+      CheckArrayType(IList, DeclType, Zero, SubobjectIsDesignatorContext, Index);
+    }
     else
       assert(0 && "Aggregate that isn't a function or array?!");
   } else if (DeclType->isVoidType() || DeclType->isFunctionType()) {
@@ -239,6 +245,8 @@ void InitListChecker::CheckVectorType(InitListExpr *IList, QualType DeclType,
 }
 
 void InitListChecker::CheckArrayType(InitListExpr *IList, QualType &DeclType, 
+                                     llvm::APSInt elementIndex,
+                                     bool SubobjectIsDesignatorContext, 
                                      unsigned &Index) {
   // Check for the special-case of initializing an array with a string.
   if (Index < IList->getNumInits()) {
@@ -263,7 +271,6 @@ void InitListChecker::CheckArrayType(InitListExpr *IList, QualType &DeclType,
 
   // FIXME: Will 32 bits always be enough? I hope so.
   const unsigned ArraySizeBits = 32;
-  llvm::APSInt elementIndex(ArraySizeBits, 0);
 
   // We might know the maximum number of elements in advance.
   llvm::APSInt maxElements(ArraySizeBits, 0);
@@ -279,16 +286,25 @@ void InitListChecker::CheckArrayType(InitListExpr *IList, QualType &DeclType,
   while (Index < IList->getNumInits()) {
     Expr *Init = IList->getInit(Index);
     if (DesignatedInitExpr *DIE = dyn_cast<DesignatedInitExpr>(Init)) {
-      // C99 6.7.8p17:
+      // If we're not the subobject that matches up with the '{' for
-      //   [...] In contrast, a designation causes the following
+      // the designator, we shouldn't be handling the
-      //   initializer to begin initialization of the subobject
+      // designator. Return immediately.
-      //   described by the designator. 
+      if (!SubobjectIsDesignatorContext)
-      FieldDecl *DesignatedField = 0;
+        return;
-      if (CheckDesignatedInitializer(IList, DIE, DeclType, DesignatedField, 
+
-                                     elementIndex, Index))
+      // Handle this designated initializer. elementIndex will be
-        hadError = true;
+      // updated to be the next array element we'll initialize.
+      if (CheckDesignatedInitializer(IList, DIE, DIE->designators_begin(), 
+                                     DeclType, 0, &elementIndex, Index)) {
+        hadError = true;
+        continue;
+      }
+
+      // If the array is of incomplete type, keep track of the number of
+      // elements in the initializer.
+      if (!maxElementsKnown && elementIndex > maxElements)
+        maxElements = elementIndex;
 
-      ++elementIndex;
       continue;
     }
 
@@ -324,6 +340,8 @@ void InitListChecker::CheckArrayType(InitListExpr *IList, QualType &DeclType,
 
 void InitListChecker::CheckStructUnionTypes(InitListExpr *IList, 
                                             QualType DeclType, 
+                                            RecordDecl::field_iterator Field,
+                                            bool SubobjectIsDesignatorContext, 
                                             unsigned &Index) {
   RecordDecl* structDecl = DeclType->getAsRecordType()->getDecl();
     
@@ -338,30 +356,23 @@ void InitListChecker::CheckStructUnionTypes(InitListExpr *IList,
   // because an error should get printed out elsewhere. It might be
   // worthwhile to skip over the rest of the initializer, though.
   RecordDecl *RD = DeclType->getAsRecordType()->getDecl();
-  RecordDecl::field_iterator Field = RD->field_begin(), 
+  RecordDecl::field_iterator FieldEnd = RD->field_end();
-                          FieldEnd = RD->field_end();
   while (Index < IList->getNumInits()) {
     Expr *Init = IList->getInit(Index);
 
     if (DesignatedInitExpr *DIE = dyn_cast<DesignatedInitExpr>(Init)) {
-      // C99 6.7.8p17:
+      // If we're not the subobject that matches up with the '{' for
-      //   [...] In contrast, a designation causes the following
+      // the designator, we shouldn't be handling the
-      //   initializer to begin initialization of the subobject
+      // designator. Return immediately.
-      //   described by the designator. Initialization then continues
+      if (!SubobjectIsDesignatorContext)
-      //   forward in order, beginning with the next subobject after
+        return;
-      //   that described by the designator. 
+
-      FieldDecl *DesignatedField = 0;
+      // Handle this designated initializer. Field will be updated to
-      llvm::APSInt LastElement;
+      // the next field that we'll be initializing.
-      if (CheckDesignatedInitializer(IList, DIE, DeclType, DesignatedField, 
+      if (CheckDesignatedInitializer(IList, DIE, DIE->designators_begin(), 
-                                     LastElement, Index)) {
+                                     DeclType, &Field, 0, Index))
-        hadError = true;
+        hadError = true;
-        continue;
-      }
 
-      Field = RecordDecl::field_iterator(
-                           DeclContext::decl_iterator(DesignatedField),
-                           DeclType->getAsRecordType()->getDecl()->decls_end());
-      ++Field;
       continue;
     }
 
@@ -411,136 +422,177 @@ void InitListChecker::CheckStructUnionTypes(InitListExpr *IList,
 /// @param DeclType  The type of the "current object" (C99 6.7.8p17),
 /// into which the designation in @p DIE should refer.
 ///
-/// @param DesignatedField  If the first designator in @p DIE is a field,
+/// @param NextField  If non-NULL and the first designator in @p DIE is
-/// this will be set to the field declaration corresponding to the
+/// a field, this will be set to the field declaration corresponding
-/// field named by the designator.
+/// to the field named by the designator.
 ///
-/// @param DesignatedIndex  If the first designator in @p DIE is an
+/// @param NextElementIndex  If non-NULL and the first designator in @p
-/// array designator or GNU array-range designator, this will be set
+/// DIE is an array designator or GNU array-range designator, this
-/// to the last index initialized by this designator.
+/// will be set to the last index initialized by this designator.
 ///
 /// @param Index  Index into @p IList where the designated initializer
 /// @p DIE occurs.
 ///
 /// @returns true if there was an error, false otherwise.
-bool InitListChecker::CheckDesignatedInitializer(InitListExpr *IList,
+bool 
-                                                 DesignatedInitExpr *DIE, 
+InitListChecker::CheckDesignatedInitializer(InitListExpr *IList,
-                                                 QualType DeclType,
+                                      DesignatedInitExpr *DIE, 
-                                                 FieldDecl *&DesignatedField, 
+                                      DesignatedInitExpr::designators_iterator D,
-                                                 llvm::APSInt &DesignatedIndex,
+                                      QualType &CurrentObjectType,
-                                                 unsigned &Index) {
+                                      RecordDecl::field_iterator *NextField,
-  // DeclType is always the type of the "current object" (C99 6.7.8p17).
+                                      llvm::APSInt *NextElementIndex,
+                                      unsigned &Index) {
+  bool IsFirstDesignator = (D == DIE->designators_begin());
 
-  for (DesignatedInitExpr::designators_iterator D = DIE->designators_begin(),
+  if (D == DIE->designators_end()) {
-                                             DEnd = DIE->designators_end();
+    // Check the actual initialization for the designated object type.
-       D != DEnd; ++D) {
+    bool prevHadError = hadError;
-    if (D->isFieldDesignator()) {
+    CheckSubElementType(IList, CurrentObjectType, DIE->getInit(), Index);
-      // C99 6.7.8p7:
+    return hadError && !prevHadError;
-      //
-      //   If a designator has the form
-      //
-      //      . identifier
-      //
-      //   then the current object (defined below) shall have
-      //   structure or union type and the identifier shall be the
-      //   name of a member of that type. 
-      const RecordType *RT = DeclType->getAsRecordType();
-      if (!RT) {
-        SemaRef->Diag(DIE->getSourceRange().getBegin(), 
-                      diag::err_field_designator_non_aggr)
-          << SemaRef->getLangOptions().CPlusPlus << DeclType;
-        ++Index;
-        return true;
-      }
-
-      IdentifierInfo *FieldName = D->getFieldName();
-      DeclContext::lookup_result Lookup = RT->getDecl()->lookup(FieldName);
-      FieldDecl *ThisField = 0;
-      if (Lookup.first == Lookup.second) {
-        // Lookup did not find anything with this name.
-        SemaRef->Diag(D->getFieldLoc(), diag::err_field_designator_unknown)
-          << FieldName << DeclType;
-      } else if (isa<FieldDecl>(*Lookup.first)) {
-        // Name lookup found a field.
-        ThisField = cast<FieldDecl>(*Lookup.first);
-        // FIXME: Make sure this isn't a field in an anonymous
-        // struct/union.
-      } else {
-        // Name lookup found something, but it wasn't a field.
-        SemaRef->Diag(D->getFieldLoc(), diag::err_field_designator_nonfield)
-          << FieldName;
-        SemaRef->Diag((*Lookup.first)->getLocation(), 
-                      diag::note_field_designator_found);
-      }
-
-      if (!ThisField) {
-        ++Index;
-        return true;
-      }
-        
-      // Update the designator with the field declaration.
-      D->setField(ThisField);
-      
-      if (D == DIE->designators_begin())
-        DesignatedField = ThisField;
-
-      // The current object is now the type of this field.
-      DeclType = ThisField->getType();
-    } else {
-      // C99 6.7.8p6:
-      //
-      //   If a designator has the form
-      //
-      //      [ constant-expression ]
-      //
-      //   then the current object (defined below) shall have array
-      //   type and the expression shall be an integer constant
-      //   expression. If the array is of unknown size, any
-      //   nonnegative value is valid.
-      const ArrayType *AT = SemaRef->Context.getAsArrayType(DeclType);
-      if (!AT) {
-        SemaRef->Diag(D->getLBracketLoc(), diag::err_array_designator_non_array)
-          << DeclType;
-        ++Index;
-        return true;
-      }
-
-      Expr *IndexExpr = 0;
-      llvm::APSInt ThisIndex;
-      if (D->isArrayDesignator())
-        IndexExpr = DIE->getArrayIndex(*D);
-      else {
-        assert(D->isArrayRangeDesignator() && "Need array-range designator");
-        IndexExpr = DIE->getArrayRangeEnd(*D);
-      }
-
-      bool ConstExpr 
-        = IndexExpr->isIntegerConstantExpr(ThisIndex, SemaRef->Context);
-      assert(ConstExpr && "Expression must be constant"); (void)ConstExpr;
-        
-      if (isa<ConstantArrayType>(AT)) {
-        llvm::APSInt MaxElements(cast<ConstantArrayType>(AT)->getSize(), false);
-        if (ThisIndex >= MaxElements) {
-          SemaRef->Diag(IndexExpr->getSourceRange().getBegin(),
-                        diag::err_array_designator_too_large)
-            << ThisIndex.toString(10) << MaxElements.toString(10);
-          ++Index;
-          return true;
-        }
-      }
-
-      if (D == DIE->designators_begin())
-        DesignatedIndex = ThisIndex;
-
-      // The current object is now the element type of this array.
-      DeclType = AT->getElementType();
-    }
   }
 
-  // Check the actual initialization for the designated object type.
+  if (D->isFieldDesignator()) {
+    // C99 6.7.8p7:
+    //
+    //   If a designator has the form
+    //
+    //      . identifier
+    //
+    //   then the current object (defined below) shall have
+    //   structure or union type and the identifier shall be the
+    //   name of a member of that type. 
+    const RecordType *RT = CurrentObjectType->getAsRecordType();
+    if (!RT) {
+      SourceLocation Loc = D->getDotLoc();
+      if (Loc.isInvalid())
+        Loc = D->getFieldLoc();
+      SemaRef->Diag(Loc, diag::err_field_designator_non_aggr)
+        << SemaRef->getLangOptions().CPlusPlus << CurrentObjectType;
+      ++Index;
+      return true;
+    }
+
+    IdentifierInfo *FieldName = D->getFieldName();
+    DeclContext::lookup_result Lookup = RT->getDecl()->lookup(FieldName);
+    FieldDecl *DesignatedField = 0;
+    if (Lookup.first == Lookup.second) {
+      // Lookup did not find anything with this name.
+      SemaRef->Diag(D->getFieldLoc(), diag::err_field_designator_unknown)
+        << FieldName << CurrentObjectType;
+    } else if (isa<FieldDecl>(*Lookup.first)) {
+      // Name lookup found a field.
+      DesignatedField = cast<FieldDecl>(*Lookup.first);
+      // FIXME: Make sure this isn't a field in an anonymous
+      // struct/union.
+    } else {
+      // Name lookup found something, but it wasn't a field.
+      SemaRef->Diag(D->getFieldLoc(), diag::err_field_designator_nonfield)
+        << FieldName;
+      SemaRef->Diag((*Lookup.first)->getLocation(), 
+                    diag::note_field_designator_found);
+    }
+
+    if (!DesignatedField) {
+      ++Index;
+      return true;
+    }
+        
+    // Update the designator with the field declaration.
+    D->setField(DesignatedField);
+      
+    // Recurse to check later designated subobjects.
+    QualType FieldType = DesignatedField->getType();
+    if (CheckDesignatedInitializer(IList, DIE, ++D, FieldType, 0, 0, Index))
+      return true;
+
+    // Find the position of the next field to be initialized in this
+    // subobject.
+    RecordDecl::field_iterator Field(DeclContext::decl_iterator(DesignatedField),
+                                     RT->getDecl()->decls_end());
+    ++Field;
+
+    // If this the first designator, our caller will continue checking
+    // the rest of this struct/class/union subobject.
+    if (IsFirstDesignator) {
+      if (NextField)
+        *NextField = Field;
+      return false;
+    }
+
+    // Check the remaining fields within this class/struct/union subobject.
+    bool prevHadError = hadError;
+    CheckStructUnionTypes(IList, CurrentObjectType, Field, false, Index);
+    return hadError && !prevHadError;
+  }
+
+  // C99 6.7.8p6:
+  //
+  //   If a designator has the form
+  //
+  //      [ constant-expression ]
+  //
+  //   then the current object (defined below) shall have array
+  //   type and the expression shall be an integer constant
+  //   expression. If the array is of unknown size, any
+  //   nonnegative value is valid.
+  //
+  // Additionally, cope with the GNU extension that permits
+  // designators of the form
+  //
+  //      [ constant-expression ... constant-expression ]
+  const ArrayType *AT = SemaRef->Context.getAsArrayType(CurrentObjectType);
+  if (!AT) {
+    SemaRef->Diag(D->getLBracketLoc(), diag::err_array_designator_non_array)
+      << CurrentObjectType;
+    ++Index;
+    return true;
+  }
+
+  Expr *IndexExpr = 0;
+  llvm::APSInt DesignatedIndex;
+  if (D->isArrayDesignator())
+    IndexExpr = DIE->getArrayIndex(*D);
+  else {
+    assert(D->isArrayRangeDesignator() && "Need array-range designator");
+    IndexExpr = DIE->getArrayRangeEnd(*D);
+  }
+
+  bool ConstExpr 
+    = IndexExpr->isIntegerConstantExpr(DesignatedIndex, SemaRef->Context);
+  assert(ConstExpr && "Expression must be constant"); (void)ConstExpr;
+  
+  if (isa<ConstantArrayType>(AT)) {
+    llvm::APSInt MaxElements(cast<ConstantArrayType>(AT)->getSize(), false);
+    if (DesignatedIndex >= MaxElements) {
+      SemaRef->Diag(IndexExpr->getSourceRange().getBegin(),
+                    diag::err_array_designator_too_large)
+        << DesignatedIndex.toString(10) << MaxElements.toString(10)
+        << IndexExpr->getSourceRange();
+      ++Index;
+      return true;
+    }
+  }
+  
+  // Recurse to check later designated subobjects.
+  QualType ElementType = AT->getElementType();
+  if (CheckDesignatedInitializer(IList, DIE, ++D, ElementType, 0, 0, Index))
+    return true;
+
+  // Move to the next index in the array that we'll be initializing.
+  ++DesignatedIndex;
+
+  // If this the first designator, our caller will continue checking
+  // the rest of this array subobject.
+  if (IsFirstDesignator) {
+    if (NextElementIndex)
+      *NextElementIndex = DesignatedIndex;
+    return false;
+  }
+    
+  // Check the remaining elements within this array subobject.
   bool prevHadError = hadError;
-  CheckSubElementType(IList, DeclType, DIE->getInit(), Index);
+  CheckArrayType(IList, CurrentObjectType, DesignatedIndex, true, Index);
-  return hadError && !prevHadError;
+  return hadError && !prevHadError;  
 }
 
 /// Check that the given Index expression is a valid array designator

clang/test/Sema/designated-initializers.c

@@ -1,5 +1,9 @@
 // RUN: clang -fsyntax-only -verify %s
 
+int complete_array_from_init[] = { 1, 2, [10] = 5, 1, 2, [5] = 2, 6 };
+
+int complete_array_from_init_check[((sizeof(complete_array_from_init) / sizeof(int)) == 13)? 1 : -1];
+
 int iarray[10] = {
   [0] = 1,
   [1 ... 5] = 2,
@@ -11,6 +15,9 @@ int iarray[10] = {
 
 int iarray2[10] = {
   [10] = 1, // expected-error{{array designator index (10) exceeds array bounds (10)}}
+};
+
+int iarray3[10] = {
   [5 ... 12] = 2 // expected-error{{array designator index (12) exceeds array bounds (10)}}
 };
 
@@ -23,7 +30,9 @@ struct point p1 = {
   .y = 1.0, 
   x: 2.0,
   .a = 4.0, // expected-error{{field designator 'a' does not refer to any field in type 'struct point'}}
+};
 
+struct point p2 = {
   [1] = 1.0 // expected-error{{array designator cannot initialize non-array type}}
 };
 
@@ -31,9 +40,15 @@ struct point array[10] = {
   [0].x = 1.0,
   [1].y = 2.0,
   [2].z = 3.0, // expected-error{{field designator 'z' does not refer to any field in type 'struct point'}}
+};
+
+struct point array2[10] = {
   [10].x = 2.0, // expected-error{{array designator index (10) exceeds array bounds (10)}}
   [4 ... 5].y = 2.0,
-  [4 ... 6] = { .x = 3, .y = 4.0 },
+  [4 ... 6] = { .x = 3, .y = 4.0 }
+};
+
+struct point array3[10] = {
   .x = 5 // expected-error{{field designator cannot initialize a non-struct, non-union type}}
 };
 
@@ -76,3 +91,24 @@ struct translator {
   .wonky = { 0 }
 };
 
+int anint;
+struct {int x,*y;} z[] = {[0].x = 2, &z[0].x};
+
+struct outer { struct inner { int x, *y; } in, *inp; } zz[] = {
+  [0].in.x = 2, &zz[0].in.x, &zz[0].in,
+  0, &anint, &zz[1].in,
+  [3].in = { .y = &anint, .x = 17 },
+  [7].in.y = &anint, &zz[0].in,
+  [4].in.y = &anint, [5].in.x = 12
+};
+
+int zz_sizecheck[sizeof(zz) / sizeof(struct outer) == 8? 1 : -1 ];
+
+struct disklabel_ops {
+  struct {} type;
+  int labelsize;
+};
+
+struct disklabel_ops disklabel64_ops = {
+  .labelsize = sizeof(struct disklabel_ops)
+};