On Wed, Jan 21, 2015 at 9:06 AM, Justin Bogner <[email protected]> wrote:
> David Majnemer <[email protected]> writes: > > Author: majnemer > > Date: Wed Jan 21 04:54:38 2015 > > New Revision: 226653 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=226653&view=rev > > Log: > > AST: Don't ignore alignas on EnumDecls when calculating alignment > > > > We didn't consider any alignment attributes on an EnumDecl when > > calculating alignment. > > > > While we are here, ignore alignment specifications on typedef types if > > one is used as the underlying type. Otherwise, weird things happen: > > > > enum Y : int; > > Y y; > > > > typedef int __attribute__((aligned(64))) u; > > enum Y : u {}; > > > > What is the alignment of 'Y'? It would be more consistent with the > > overall design of enums with fixed underlying types to consider the > > underlying type's UnqualifiedDesugaredType. > > Is it more consistent? I think so. The standard forbids: typedef int alignas(16) Ty; enum : int alignas(4) {}; we error with: 'alignas' attribute cannot be applied to types The standard is pretty clear that alignment isn't a property of types. We would also match the behavior of MSVC and ICC, they also ignore the alignment of the underlying type. > What about the case without the forward > declaration, or if the forward declaration also specifies the alignment? > Then what do we do with: typedef int __attribute__((aligned(8))) Ty1; typedef int __attribute__((aligned(16))) Ty2; enum alignas(16) : Ty1 {}; enum alignas(8) : Ty2 {}; What alignment would Ty1 and Ty2 have? The max of all possible alignments? Just the alignment on the EnumDecl? > It's not really clear what the programmer's intent is in these cases > anyway, maybe we could warn that they seem to be doing something silly? > I think a warning would be a fine idea. > > > This fixes PR22279. > > > > Modified: > > cfe/trunk/lib/AST/ASTContext.cpp > > cfe/trunk/lib/Sema/SemaDeclAttr.cpp > > cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.align/p5.cpp > > cfe/trunk/test/Sema/align-x86.c > > > > Modified: cfe/trunk/lib/AST/ASTContext.cpp > > URL: > > > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=226653&r1=226652&r2=226653&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/AST/ASTContext.cpp (original) > > +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Jan 21 04:54:38 2015 > > @@ -1670,8 +1670,16 @@ TypeInfo ASTContext::getTypeInfoImpl(con > > break; > > } > > > > - if (const EnumType *ET = dyn_cast<EnumType>(TT)) > > - return getTypeInfo(ET->getDecl()->getIntegerType()); > > + if (const EnumType *ET = dyn_cast<EnumType>(TT)) { > > + const EnumDecl *ED = ET->getDecl(); > > + TypeInfo Info = > > + > getTypeInfo(ED->getIntegerType()->getUnqualifiedDesugaredType()); > > + if (unsigned AttrAlign = ED->getMaxAlignment()) { > > + Info.Align = AttrAlign; > > + Info.AlignIsRequired = true; > > + } > > + return Info; > > + } > > > > const RecordType *RT = cast<RecordType>(TT); > > const ASTRecordLayout &Layout = getASTRecordLayout(RT->getDecl()); > > @@ -1786,6 +1794,8 @@ unsigned ASTContext::getPreferredTypeAli > > T = T->getBaseElementTypeUnsafe(); > > if (const ComplexType *CT = T->getAs<ComplexType>()) > > T = CT->getElementType().getTypePtr(); > > + if (const EnumType *ET = T->getAs<EnumType>()) > > + T = ET->getDecl()->getIntegerType().getTypePtr(); > > if (T->isSpecificBuiltinType(BuiltinType::Double) || > > T->isSpecificBuiltinType(BuiltinType::LongLong) || > > T->isSpecificBuiltinType(BuiltinType::ULongLong)) > > > > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=226653&r1=226652&r2=226653&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) > > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jan 21 04:54:38 2015 > > @@ -2953,12 +2953,15 @@ void Sema::AddAlignedAttr(SourceRange At > > void Sema::CheckAlignasUnderalignment(Decl *D) { > > assert(D->hasAttrs() && "no attributes on decl"); > > > > - QualType Ty; > > - if (ValueDecl *VD = dyn_cast<ValueDecl>(D)) > > - Ty = VD->getType(); > > - else > > - Ty = Context.getTagDeclType(cast<TagDecl>(D)); > > - if (Ty->isDependentType() || Ty->isIncompleteType()) > > + QualType UnderlyingTy, DiagTy; > > + if (ValueDecl *VD = dyn_cast<ValueDecl>(D)) { > > + UnderlyingTy = DiagTy = VD->getType(); > > + } else { > > + UnderlyingTy = DiagTy = Context.getTagDeclType(cast<TagDecl>(D)); > > + if (EnumDecl *ED = dyn_cast<EnumDecl>(D)) > > + UnderlyingTy = ED->getIntegerType(); > > + } > > + if (DiagTy->isDependentType() || DiagTy->isIncompleteType()) > > return; > > > > // C++11 [dcl.align]p5, C11 6.7.5/4: > > @@ -2977,10 +2980,10 @@ void Sema::CheckAlignasUnderalignment(De > > > > if (AlignasAttr && Align) { > > CharUnits RequestedAlign = Context.toCharUnitsFromBits(Align); > > - CharUnits NaturalAlign = Context.getTypeAlignInChars(Ty); > > + CharUnits NaturalAlign = Context.getTypeAlignInChars(UnderlyingTy); > > if (NaturalAlign > RequestedAlign) > > Diag(AlignasAttr->getLocation(), diag::err_alignas_underaligned) > > - << Ty << (unsigned)NaturalAlign.getQuantity(); > > + << DiagTy << (unsigned)NaturalAlign.getQuantity(); > > } > > } > > > > > > Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.align/p5.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.align/p5.cpp?rev=226653&r1=226652&r2=226653&view=diff > > > ============================================================================== > > --- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.align/p5.cpp (original) > > +++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.align/p5.cpp Wed Jan 21 > 04:54:38 2015 > > @@ -15,6 +15,12 @@ enum alignas(1) E1 {}; // expected-error > > enum alignas(1) E2 : char {}; // ok > > enum alignas(4) E3 { e3 = 0 }; // ok > > enum alignas(4) E4 { e4 = 1ull << 33 }; // expected-error {{requested > alignment is less than minimum alignment of 8 for type 'E4'}} > > +enum alignas(8) E5 {}; > > +static_assert(alignof(E5) == 8, ""); > > + > > +typedef __attribute__((aligned(16))) int IntAlign16; > > +enum E6 : IntAlign16 {}; > > +static_assert(alignof(E6) == 4, ""); > > > > struct S1 { > > alignas(8) int n; > > > > Modified: cfe/trunk/test/Sema/align-x86.c > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/align-x86.c?rev=226653&r1=226652&r2=226653&view=diff > > > ============================================================================== > > --- cfe/trunk/test/Sema/align-x86.c (original) > > +++ cfe/trunk/test/Sema/align-x86.c Wed Jan 21 04:54:38 2015 > > @@ -27,6 +27,8 @@ double g6[3]; > > short chk1[__alignof__(g6) == 8 ? 1 : -1]; > > short chk2[__alignof__(double[3]) == 8 ? 1 : -1]; > > > > +enum { x = 18446744073709551615ULL } g7; > > +short chk1[__alignof__(g7) == 8 ? 1 : -1]; > > > > // PR5637 > > > > > > > > _______________________________________________ > > 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
