r215372 - Sema: Properly perform lookup when acting on fields for desig inits
David Majnemer
david.majnemer at gmail.com
Fri Aug 22 18:58:13 PDT 2014
Fixed in r216313.
On Thu, Aug 21, 2014 at 11:32 AM, Richard Smith <richard at metafoo.co.uk>
wrote:
> On Mon, Aug 11, 2014 at 11:33 AM, David Majnemer <david.majnemer at gmail.com
> > 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140822/bb37a4aa/attachment.html>
More information about the cfe-commits
mailing list