<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">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>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> </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">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><br></div></div>