<div dir="ltr">Fixed in r216313.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 21, 2014 at 11:32 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Mon, Aug 11, 2014 at 11:33 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: majnemer<br>
Date: Mon Aug 11 13:33:59 2014<br>
New Revision: 215372<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215372&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=215372&view=rev</a><br>
Log:<br>
Sema: Properly perform lookup when acting on fields for desig inits<br>
<br>
Clang used a custom implementation of lookup when handling designated<br>
initializers. The custom code was not particularly optimized and relied<br>
on standard lookup for typo-correction anyway.<br>
<br>
This custom code has to go, it doesn't properly support MSVC-style<br>
anonymous structs embedded inside other records; replace it with the<br>
typo-correction path.<br>
<br>
This has the side effect of speeding up semantic handling of the fields<br>
for a designated initializer while simplifying the code at the same<br>
time.<br>
<br>
This fixes PR20573.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D4839" target="_blank">http://reviews.llvm.org/D4839</a><br>
<br>
Modified:<br>
  cfe/trunk/lib/Sema/SemaInit.cpp<br>
  cfe/trunk/test/Sema/MicrosoftExtensions.c<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaInit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=215372&r1=215371&r2=215372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=215372&r1=215371&r2=215372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Aug 11 13:33:59 2014<br>
@@ -1734,24 +1734,6 @@ static void ExpandAnonymousFieldDesignat<br>
             &Replacements[0] + Replacements.size());<br>
 }<br>
<br>
-/// \brief Given an implicit anonymous field, search the IndirectField that<br>
-///Â corresponds to FieldName.<br>
-static IndirectFieldDecl *FindIndirectFieldDesignator(FieldDecl *AnonField,<br>
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â IdentifierInfo *FieldName) {<br>
-Â if (!FieldName)<br>
-Â Â return nullptr;<br>
-<br>
-Â assert(AnonField->isAnonymousStructOrUnion());<br>
-Â Decl *NextDecl = AnonField->getNextDeclInContext();<br>
-Â while (IndirectFieldDecl *IF =<br>
-Â Â Â Â Â dyn_cast_or_null<IndirectFieldDecl>(NextDecl)) {<br>
-Â Â if (FieldName == IF->getAnonField()->getIdentifier())<br>
-Â Â Â return IF;<br>
-Â Â NextDecl = NextDecl->getNextDeclInContext();<br>
-Â }<br>
-Â return nullptr;<br>
-}<br>
-<br>
 static DesignatedInitExpr *CloneDesignatedInitExpr(Sema &SemaRef,<br>
                          DesignatedInitExpr *DIE) {<br>
  unsigned NumIndexExprs = DIE->getNumSubExprs() - 1;<br>
@@ -1892,103 +1874,68 @@ InitListChecker::CheckDesignatedInitiali<br>
    return true;<br>
   }<br>
<br>
-Â Â // Note: we perform a linear search of the fields here, despite<br>
-Â Â // the fact that we have a faster lookup method, because we always<br>
-Â Â // need to compute the field's index.<br>
   FieldDecl *KnownField = D->getField();<br>
-Â Â IdentifierInfo *FieldName = D->getFieldName();<br>
-Â Â unsigned FieldIndex = 0;<br>
-Â Â RecordDecl::field_iterator<br>
-Â Â Â Field = RT->getDecl()->field_begin(),<br>
-Â Â Â FieldEnd = RT->getDecl()->field_end();<br>
-Â Â for (; Field != FieldEnd; ++Field) {<br>
-Â Â Â if (Field->isUnnamedBitfield())<br>
-Â Â Â Â continue;<br>
-<br>
-Â Â Â // If we find a field representing an anonymous field, look in the<br>
-Â Â Â // IndirectFieldDecl that follow for the designated initializer.<br>
-Â Â Â if (!KnownField && Field->isAnonymousStructOrUnion()) {<br>
-Â Â Â Â if (IndirectFieldDecl *IF =<br>
-Â Â Â Â Â Â FindIndirectFieldDesignator(*Field, FieldName)) {<br>
+Â Â if (!KnownField) {<br>
+Â Â Â IdentifierInfo *FieldName = D->getFieldName();<br>
+Â Â Â DeclContext::lookup_result Lookup = RT->getDecl()->lookup(FieldName);<br>
+Â Â Â for (NamedDecl *ND : Lookup) {<br>
+Â Â Â Â if (auto *FD = dyn_cast<FieldDecl>(ND)) {<br>
+Â Â Â Â Â KnownField = FD;<br>
+Â Â Â Â Â break;<br>
+Â Â Â Â }<br>
+Â Â Â Â if (auto *IFD = dyn_cast<IndirectFieldDecl>(ND)) {<br>
      // In verify mode, don't modify the original.<br>
      if (VerifyOnly)<br>
       DIE = CloneDesignatedInitExpr(SemaRef, DIE);<br>
-Â Â Â Â Â ExpandAnonymousFieldDesignator(SemaRef, DIE, DesigIdx, IF);<br>
+Â Â Â Â Â ExpandAnonymousFieldDesignator(SemaRef, DIE, DesigIdx, IFD);<br>
      D = DIE->getDesignator(DesigIdx);<br>
+Â Â Â Â Â KnownField = cast<FieldDecl>(*IFD->chain_begin());<br>
      break;<br>
     }<br>
    }<br>
-Â Â Â if (KnownField && KnownField == *Field)<br>
-Â Â Â Â break;<br>
-Â Â Â if (FieldName && FieldName == Field->getIdentifier())<br>
-Â Â Â Â break;<br>
-<br>
-Â Â Â ++FieldIndex;<br>
-Â Â }<br>
+Â Â Â if (!KnownField) {<br>
+Â Â Â Â if (VerifyOnly) {<br>
+Â Â Â Â Â ++Index;<br>
+Â Â Â Â Â return true;Â // No typo correction when just trying this out.<br>
+Â Â Â Â }<br>
<br>
-Â Â if (Field == FieldEnd) {<br>
-Â Â Â if (VerifyOnly) {<br>
-Â Â Â Â ++Index;<br>
-Â Â Â Â return true; // No typo correction when just trying this out.<br>
-Â Â Â }<br>
+Â Â Â Â // Name lookup found something, but it wasn't a field.<br>
+Â Â Â Â if (!Lookup.empty()) {<br>
+Â Â Â Â Â SemaRef.Diag(D->getFieldLoc(), diag::err_field_designator_nonfield)<br>
+Â Â Â Â Â Â << FieldName;<br>
+Â Â Â Â Â SemaRef.Diag(Lookup.front()->getLocation(),<br>
+Â Â Â Â Â Â Â Â Â Â Â Â diag::note_field_designator_found);<br>
+Â Â Â Â Â ++Index;<br>
+Â Â Â Â Â return true;<br>
+Â Â Â Â }<br>
<br>
-Â Â Â // There was no normal field in the struct with the designated<br>
-Â Â Â // name. Perform another lookup for this name, which may find<br>
-Â Â Â // something that we can't designate (e.g., a member function),<br>
-Â Â Â // may find nothing, or may find a member of an anonymous<br>
-Â Â Â // struct/union.<br>
-Â Â Â DeclContext::lookup_result Lookup = RT->getDecl()->lookup(FieldName);<br>
-Â Â Â FieldDecl *ReplacementField = nullptr;<br>
-Â Â Â if (Lookup.empty()) {<br>
-Â Â Â Â // Name lookup didn't find anything. Determine whether this<br>
-Â Â Â Â // was a typo for another field name.<br>
+Â Â Â Â // Name lookup didn't find anything.<br>
+Â Â Â Â // Determine whether this was a typo for another field name.<br>
     FieldInitializerValidatorCCC Validator(RT->getDecl());<br>
     if (TypoCorrection Corrected = SemaRef.CorrectTypo(<br>
         DeclarationNameInfo(FieldName, D->getFieldLoc()),<br>
-Â Â Â Â Â Â Â Â Sema::LookupMemberName, /*Scope=*/ nullptr, /*SS=*/ nullptr,<br>
+Â Â Â Â Â Â Â Â Sema::LookupMemberName, /*Scope=*/nullptr, /*SS=*/nullptr,<br>
         Validator, Sema::CTK_ErrorRecovery, RT->getDecl())) {<br>
      SemaRef.diagnoseTypo(<br>
        Corrected,<br>
        SemaRef.PDiag(diag::err_field_designator_unknown_suggest)<br>
-Â Â Â Â Â Â Â Â Â << FieldName << CurrentObjectType);<br>
-Â Â Â Â Â ReplacementField = Corrected.getCorrectionDeclAs<FieldDecl>();<br>
+Â Â Â Â Â Â Â Â << FieldName << CurrentObjectType);<br>
+Â Â Â Â Â KnownField = Corrected.getCorrectionDeclAs<FieldDecl>();<br>
      hadError = true;<br>
     } else {<br>
+Â Â Â Â Â // Typo correction didn't find anything.<br>
      SemaRef.Diag(D->getFieldLoc(), diag::err_field_designator_unknown)<br>
       << FieldName << CurrentObjectType;<br>
      ++Index;<br>
      return true;<br>
     }<br>
    }<br>
-<br>
-Â Â Â if (!ReplacementField) {<br>
-Â Â Â Â // Name lookup found something, but it wasn't a field.<br>
-Â Â Â Â SemaRef.Diag(D->getFieldLoc(), diag::err_field_designator_nonfield)<br>
-Â Â Â Â Â << FieldName;<br>
-Â Â Â Â SemaRef.Diag(Lookup.front()->getLocation(),<br>
-Â Â Â Â Â Â Â Â Â Â Â diag::note_field_designator_found);<br>
-Â Â Â Â ++Index;<br>
-Â Â Â Â return true;<br>
-Â Â Â }<br>
-<br>
-Â Â Â if (!KnownField) {<br>
-Â Â Â Â // The replacement field comes from typo correction; find it<br>
-Â Â Â Â // in the list of fields.<br>
-Â Â Â Â FieldIndex = 0;<br>
-Â Â Â Â Field = RT->getDecl()->field_begin();<br>
-Â Â Â Â for (; Field != FieldEnd; ++Field) {<br>
-Â Â Â Â Â if (Field->isUnnamedBitfield())<br>
-Â Â Â Â Â Â continue;<br>
-<br>
-Â Â Â Â Â if (ReplacementField == *Field ||<br>
-Â Â Â Â Â Â Â Field->getIdentifier() == ReplacementField->getIdentifier())<br>
-Â Â Â Â Â Â break;<br>
-<br>
-Â Â Â Â Â ++FieldIndex;<br>
-Â Â Â Â }<br>
-Â Â Â }<br>
   }<br>
<br>
+Â Â unsigned FieldIndex = KnownField->getFieldIndex();<br></blockquote><div><br></div></div></div><div>Ugh, this is not quite correct, because getFieldIndex() includes unnamed bitfields (which don't get an entry in an InitListExpr). So</div>
<div><br></div><div>Â Â struct S { int : 32; int n; } s = { .n = 123 };<br></div><div><br></div><div>... now initializes 'n' to 0, not to 123 (and produces a slightly broken AST). Looks like we need to bring back the O(n) loop just to compute this index. =(</div>
<div class="">
<div>Â </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+Â Â RecordDecl::field_iterator Field =<br>
+Â Â Â Â RecordDecl::field_iterator(DeclContext::decl_iterator(KnownField));<br>
+<br>
   // All of the fields of a union are located at the same place in<br>
   // the initializer list.<br>
   if (RT->getDecl()->isUnion()) {<br>
<br>
Modified: cfe/trunk/test/Sema/MicrosoftExtensions.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/MicrosoftExtensions.c?rev=215372&r1=215371&r2=215372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/MicrosoftExtensions.c?rev=215372&r1=215371&r2=215372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/MicrosoftExtensions.c (original)<br>
+++ cfe/trunk/test/Sema/MicrosoftExtensions.c Mon Aug 11 13:33:59 2014<br>
@@ -39,6 +39,8 @@ struct nested2 {<br>
  NESTED1; // expected-warning {{anonymous structs are a Microsoft extension}}<br>
 };<br>
<br>
+struct nested2 PR20573 = { .a = 3 };<br>
+<br>
 struct nested3 {<br>
  long d;<br>
  struct nested4 { // expected-warning {{anonymous structs are a Microsoft extension}}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div>