[cfe-commits] [PATCH][Review request] - IndirectField + designated initializers.

[Douglas Gregor]

On Dec 12, 2010, at 7:23 PM, Francois Pichet wrote:

> More IndirectFieldDecl refactoring.
> 
> This patch deals with a dark corner of clang: anonymous field
> initialized with designated-initializers (C99 feature)
> This is C++ mixing with C99.. not sure why clang supports that but it does.

Anonymous structs and anonymous unions are GNU extensions in C (and the former is an extension in C++, too) that GCC implements, so Clang also implements it.

> Example:
> 
> struct Test {
>  union {
>      int a;
>      char b;
>  };
> };
> 
> struct Test  var = { .a = 4 };
> 
> Some explaining: currently designated anonymous fields are found via
> name look. Normal fields are found by iterating all the fields. This
> patch uses the fact that IndirectFieldDecl declarations will follow an
> anonymous implicit field to remove the special case of name lookup for
> anonymous field. With this patch there is no need to calculate the
> anonymous field position afterward. Clear enough?

This is causing Clang to crash on the test

	test/CodeGen/designated-initializers.c

This is a bit suspicious:

@@ -1386,7 +1356,15 @@

     for (; Field != FieldEnd; ++Field) {
       if (Field->isUnnamedBitfield())
         continue;
-
+      
+      // If we find a field representing an anonymous field, look in the
+      // IndirectFieldDecl that follow for the designated initializer.
+      if (!KnownField && Field->isAnonymousStructOrUnion()) {
+        if (IndirectFieldDecl *IF = FindIndirectFieldDesignator(*Field, FieldName))
+          ExpandAnonymousFieldDesignator(SemaRef, DIE, DesigIdx, IF);
+          D = DIE->getDesignator(DesigIdx);
+          break;
+      }

since there's no { } around the body of the inner if.

+/// \brief Given an implicit anonymous field, search the IndirectField that 
+///  corresponds to FieldName.
+static IndirectFieldDecl *FindIndirectFieldDesignator(FieldDecl *AnonField,
+                                                 IdentifierInfo *FieldName) {
+  assert(AnonField->isAnonymousStructOrUnion());
+  Decl *NextDecl = AnonField->getNextDeclInContext();
+  while (IndirectFieldDecl *IF = dyn_cast<IndirectFieldDecl>(NextDecl)) {
+    if (FieldName && FieldName == IF->getAnonField()->getIdentifier())
+      return IF;
+    NextDecl = NextDecl->getNextDeclInContext();
   }
-
-  assert(false && "Unable to find anonymous struct/union field");
 }

There's no "return 0;" at the end here (?)

With those changes, the patch seems fine to me.

	- Doug