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