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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:2154
+  if (uint64_t Result = SubstTypeVisitor(Ctx, SubType->getReplacementType())
+                            .TryEval(Attr->getAlignmentExpr())) {
+    TI.Align = Result;
----------------
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 `TypedefDecl`s, 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 `TypedefType`s 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 `AlignedAttr`s 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.


================
Comment at: clang/lib/AST/ASTContext.cpp:2577-2583
+  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;
+  }
----------------
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 `TemplateSpecializationType`s for which `isTypeAlias` is `true`... but see my other comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143533/new/

https://reviews.llvm.org/D143533