r215372 - Sema: Properly perform lookup when acting on fields for desig inits
Richard Smith
richard at metafoo.co.uk
Thu Aug 21 11:32:16 PDT 2014
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/20140821/1f804a27/attachment.html>
More information about the cfe-commits
mailing list