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