[clang] Allow gnu::aligned attribute to work with templated type alias declarations

[leonardchan]

Summary

Prior to this, the following would fail:

template <typename T>
using NaturallyAligned [[gnu::aligned(sizeof(T))]] = T;

struct S { char x[2]; };
using AlignedS = NaturallyAligned<S>;
static_assert(alignof(AlignedS) == 2);  // alignof(AlignedS) == 1

This is because clang ignores attributes on type aliases when evaluating type info and instead resolves to alignment of the replacement type T.

This patch attempts to check if the associated type declaration is an alias that contains an attribute and return whatever the value is indicated by the alignment expression.

[rsmith]

clang/lib/AST/ASTContext.cpp

2154

I don't think this approach is going to work out well. There are many cases this gets wrong, fixing them would require reinventing template substitution here, and generally we shouldn't be doing this substitution as part of alignment computation at all -- we should have TreeTransform produce the right alignment value during template instantiation and just pull it back out of the Type here. We can't really use the same approach as we do for TypedefDecls, though, because we don't instantiate a TypeAliasDecl for each use of a TypeAliasTemplateDecl, so there's nowhere to hang an instantiated attribute. But the handling for TypedefTypes is also fairly painful, so I think we should be considering an approach that makes both of them more elegant. Here's what I suggest:

• We add a new sugar-only Type subclass that represents an alignment attribute. Maybe we can model this as an AttributedType for the AlignedAttr, or maybe we create a new kind of type node.
• We translate AlignedAttrs on typedef and type alias declarations into this new kind of Type wrapping the declared type.
• We make getTypeInfoImpl special-case that type sugar node instead of special-casing TypedefType sugar.
• We make sure that TreeTransform properly instantiates the new node, in particular performing substitution within the argument.

2577–2583

Looking for a SubstTemplateTypeParm will mean that you don't handle cases like:

template<int N> using AlignedChar [[gnu::aligned(N)]] = char;

Instead, it would be better to add the special treatment to TemplateSpecializationTypes for which isTypeAlias is true... but see my other comment.

[aaron.ballman]

Adding Erich as attributes code owner.

clang/lib/AST/ASTContext.cpp

2154

I think this approach makes sense to me, but it's worth noting: the alignment attributes (vendor specific or standard) are declaration attributes and not type attributes. I think this is a design mistake; alignment is a fundamental property of a type (see http://eel.is/c++draft/basic.align#1), so I'm in favor of modeling this as a type attribute, but the standard alignas attribute does not have type semantics: http://eel.is/c++draft/dcl.align#1. Should we approach the standards committees about this or are we not concerned about alignas behavior?

(If we expect alignas to be a declaration attribute and [[gnu::aligned]] to be a type attribute, another question is whether we should split them in Attr.td.)

[erichkeane]

clang/lib/AST/ASTContext.cpp

2154

IMO, the idea of doing this via this visitor is incorrect, we should just make sure the instantiated decl of the type alias to get the attribute, we do similar things for function decls.

clang/lib/AST/ASTContext.cpp

 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/MangleNumberingContext.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/Stmt.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/Builtins.h"
if (I != MemoizedTypeInfo.end())
     return I->second;
 
   // This call can invalidate MemoizedTypeInfo[T], so we need a second lookup.
   TypeInfo TI = getTypeInfoImpl(T);
   MemoizedTypeInfo[T] = TI;
   return TI;
 }
 
+/// This class attempts to substitute the template parameter for a TypeAliasDecl
+/// with the replacement type for a SubstTemplateTypeParmType in order to create
+/// a new expression that is evaluatable (ie. not value-dependent). We need to
+/// create a new expression rather than replace componenets of an existing
+/// expression to recompute `dependence`s. Normally, this would be done via the
+/// TemplateDeclInstantiator, but that requires a reference to Sema which may
+/// not be available since the TypeInfo machinery only operates on the
+/// ASTContext.
+///
+/// NOTE: This class should be updated for various dependent expressions that
+/// can appear in an AlignedAttr's alignment expression. This can be done by
+/// adding more Visit methods.
+///
+class SubstTypeVisitor
+    : public StmtVisitor<SubstTypeVisitor, /*RetTy=*/Expr *> {
+public:
+  SubstTypeVisitor(ASTContext &Ctx, QualType ReplacementTy)
+      : Ctx(Ctx), ReplacementTy(ReplacementTy) {}
+
+  uint64_t TryEval(Expr *E) {
+    Expr *NewE = Visit(E);
+    if (!NewE || NewE->isValueDependent())
+      return 0;
+    return NewE->EvaluateKnownConstInt(Ctx).getZExtValue() * Ctx.getCharWidth();
+  }
+
+  Expr *VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) {
+    return new (Ctx) UnaryExprOrTypeTraitExpr(
+        E->getKind(), Ctx.CreateTypeSourceInfo(ReplacementTy), E->getType(),
+        E->getBeginLoc(), E->getEndLoc());
+  }
+
+  Expr *VisitDeclRefExpr(DeclRefExpr *E) {
+    if (const auto *NTPD =
+            dyn_cast<NonTypeTemplateParmDecl>(E->getFoundDecl())) {
+      if (Expr *DefaultArg = NTPD->getDefaultArgument())
+        return Visit(DefaultArg);
+    }
+    return E;
+  }
+
+  Expr *VisitUnaryOperator(UnaryOperator *E) {
+    Expr *SubExpr = Visit(E->getSubExpr());
+    if (!SubExpr)
+      return nullptr;
+    return UnaryOperator::Create(Ctx, SubExpr, E->getOpcode(), E->getType(),
+                                 E->getValueKind(), E->getObjectKind(),
+                                 E->getExprLoc(), E->canOverflow(),
+                                 E->getFPOptionsOverride());
+  }
+
+  Expr *VisitImplicitCastExpr(ImplicitCastExpr *E) {
+    Expr *SubExpr = Visit(E->getSubExpr());
+    if (!SubExpr)
+      return nullptr;
+    return ImplicitCastExpr::Create(Ctx, E->getType(), E->getCastKind(),
+                                    SubExpr, nullptr, E->getValueKind(),
+                                    E->getFPFeatures());
+  }
+
+  Expr *VisitBinaryOperator(BinaryOperator *E) {
+    Expr *LHS = Visit(E->getLHS());
+    if (!LHS)
+      return nullptr;
+    Expr *RHS = Visit(E->getRHS());
+    if (!RHS)
+      return nullptr;
+    return BinaryOperator::Create(Ctx, LHS, RHS, E->getOpcode(), E->getType(),
+                                  E->getValueKind(), E->getObjectKind(),
+                                  E->getExprLoc(), E->getFPFeatures());
+  }
+
+  Expr *VisitExpr(Expr *E) { return E; }
+
+private:
+  ASTContext &Ctx;
+  QualType ReplacementTy;
+};
+
+/// If the SubstTemplateTypeParm type points to a template alias declaration,
+/// then we can try to substitute the template parameter with the replacement
+/// type and evaluate the alignment from there. This is needed for alignment
+/// attributes where the alignment expression is not evaluatable because it has
+/// a value-dependent type. An example of this is:
+///
+///   template <typename T>
+///   using NaturallyAligned [[gnu::aligned(sizeof(T))]] = T;
+///
+/// where the expression `sizeof(T)` cannot be evaluated yet unless `T` is
+/// known. For a SubstTemplateTypeParmType, we can substitute `T` for the
+/// replacement type and try to evaluate `sizeof(T)` from here. If the
+/// expression is still not evaluatable, default to the alignment of the
+/// replacement type.
+static bool
+MaybeGetAlignForTemplateAlias(TypeInfo &TI,
+                              const SubstTemplateTypeParmType *SubType) {
+  // We only want to cover the case where the type revers to an alias with an
+  // AlignedAttr.
+  const TemplateDecl *TD;
+  if (!(TD = dyn_cast<TemplateDecl>(SubType->getAssociatedDecl())))
+    return false;
+
+  if (!TD->isTypeAlias())
+    return false;
+
+  const TypeAliasDecl *TAD;
+  if (!(TAD = dyn_cast<TypeAliasDecl>(TD->getTemplatedDecl())))
+    return false;
+
+  const AlignedAttr *Attr;
+  if (!(Attr = TAD->getAttr<AlignedAttr>()))
+    return false;
+
+  auto &Ctx = TD->getASTContext();
+
+  // Short circuit case: the expression is immediately evaluatable.
+  if (!Attr->getAlignmentExpr()->isValueDependent()) {
+    TI.Align = Attr->getAlignment(Ctx);
+    return true;
+  }
+
+  // Otherwise, try to see if we can substitute any template parameters with the
+  // replacement type.
+  if (uint64_t Result = SubstTypeVisitor(Ctx, SubType->getReplacementType())
+                            .TryEval(Attr->getAlignmentExpr())) {
+    TI.Align = Result;
+    return true;
+  }
+
+  return false;
+}
+
 /// getTypeInfoImpl - Return the size of the specified type, in bits.  This
 /// method does not work on incomplete types.
 ///
 /// FIXME: Pointers into different addr spaces could have different sizes and
 /// alignment requirements: getPointerInfo should take an AddrSpace, this
 /// should take a QualType, &c.
 TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
   uint64_t Width = 0;
case Type::Enum: {
     Width = toBits(Layout.getSize());
     Align = toBits(Layout.getAlignment());
     AlignRequirement = RD->hasAttr<AlignedAttr>()
                            ? AlignRequirementKind::RequiredByRecord
                            : AlignRequirementKind::None;
     break;
   }
 
-  case Type::SubstTemplateTypeParm:
-    return getTypeInfo(cast<SubstTemplateTypeParmType>(T)->
-                       getReplacementType().getTypePtr());
-
+  case Type::SubstTemplateTypeParm: {
+    const auto *SubType = cast<SubstTemplateTypeParmType>(T);
+    auto TI = getTypeInfo(SubType->getReplacementType().getTypePtr());
+    if (MaybeGetAlignForTemplateAlias(TI, SubType))
+      TI.AlignRequirement = AlignRequirementKind::RequiredByTypedef;
+    return TI;
+  }
   case Type::Auto:
   case Type::DeducedTemplateSpecialization: {
     const auto *A = cast<DeducedType>(T);
     assert(!A->getDeducedType().isNull() &&
            "cannot request the size of an undeduced or dependent auto type");
     return getTypeInfo(A->getDeducedType().getTypePtr());
   }
 

clang/test/SemaTemplate/type-alias-aligned.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -triple=x86_64-linux-gnu
+// expected-no-diagnostics
+
+template <typename T>
+using Aligned8 [[gnu::aligned(8)]] = T;
+
+template <typename T>
+using NaturallyAligned [[gnu::aligned(sizeof(T))]] = T;
+
+template <typename T>
+using UnderAligned [[gnu::aligned(1)]] = T;
+
+template <typename T, unsigned S = sizeof(T)>
+using NaturallyAligned2 [[gnu::aligned(S)]] = T;
+
+template <typename T>
+using UnaryAligned [[gnu::aligned(+sizeof(T))]] = T;
+
+template <typename T>
+using BinaryAligned [[gnu::aligned(sizeof(T) * 2)]] = T;
+
+struct S { unsigned x[2]; };
+static_assert(alignof(S) == 4);
+
+using Aligned8_S = Aligned8<S>;
+static_assert(alignof(Aligned8_S) == 8);
+
+using NatAligned_S = NaturallyAligned<S>;
+static_assert(alignof(NatAligned_S) == 8);
+
+using UnderAligned_S = UnderAligned<S>;
+static_assert(alignof(UnderAligned_S) == 1);
+
+using NatAligned_S2 = NaturallyAligned2<S>;
+static_assert(alignof(NatAligned_S2) == 8);
+
+using UnaryAligned_S = UnaryAligned<S>;
+static_assert(alignof(UnaryAligned_S) == 8);
+
+using BinaryAligned_S = BinaryAligned<S>;
+static_assert(alignof(BinaryAligned_S) == 16);