r226653 - AST: Don't ignore alignas on EnumDecls when calculating alignment

Wed Jan 21 10:25:35 PST 2015

On Wed, Jan 21, 2015 at 9:06 AM, Justin Bogner <mail at justinbogner.com>
wrote:

> David Majnemer <david.majnemer at gmail.com> 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
> > 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/20150121/358dce77/attachment.html>