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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
