rsmith added inline comments.
================
Comment at: clang/include/clang/AST/TemplateBase.h:421
-public:
- constexpr TemplateArgumentLocInfo() : Template({nullptr, nullptr, 0, 0}) {}
-
- TemplateArgumentLocInfo(TypeSourceInfo *TInfo) : Declarator(TInfo) {}
+ T *getTemplate() const {
+ assert(Pointer.is<T *>());
----------------
If you're going to add more uses of the name `T`, we should rename it to
something more descriptive. Maybe `TemplateTemplateArgLocInfo` or something
like that?
================
Comment at: clang/include/clang/AST/TemplateBase.h:435-441
+ T *Template = new (Ctx) T;
+ Template->Qualifier = QualifierLoc.getNestedNameSpecifier();
+ Template->QualifierLocData = QualifierLoc.getOpaqueData();
+ Template->TemplateNameLoc = TemplateNameLoc.getRawEncoding();
+ Template->EllipsisLoc = EllipsisLoc.getRawEncoding();
+ Pointer = Template;
}
----------------
I think this is just about complex enough to be worth moving to the .cpp file.
================
Comment at: clang/include/clang/AST/TemplateBase.h:444
TypeSourceInfo *getAsTypeSourceInfo() const {
- return Declarator;
+ assert(Pointer.is<TypeSourceInfo *>());
+ return Pointer.get<TypeSourceInfo *>();
----------------
This is redundant (here and below): `get` does this assert for you.
================
Comment at: clang/include/clang/AST/TemplateBase.h:43
+// the dependency.
+template <> struct PointerLikeTypeTraits<clang::Expr *> {
+ static inline void *getAsVoidPointer(clang::Expr *P) { return P; }
----------------
hokein wrote:
> sammccall wrote:
> > At first glance this is unsafe: you could have two different definitions of
> > the same specialization in different files.
> >
> > In fact it's OK, the default one can now never be instantiated, because
> > Expr.h includes this file and so anyone that can see the definition can see
> > this specialization.
> >
> > But this is subtle/fragile: at least it needs to be spelled out explicitly
> > in the comment.
> > I'd also suggest adding a static assert below the definition of Expr that
> > it is compatible with this specialization (because it is sufficiently
> > aligned).
> >
> > (I can't think of a better alternative - use of PointerUnion is a win,
> > partly *because* it validates the alignment)
> yes, exactly.
>
> do you have a better idea on how the static_assert should look like? The way
> I can think of is to add a new flag in this specialization, and use
> `static_assert(PointerLikeTypeTraits<clang::Expr *>::flag, "...")`.
This may be included from `Expr.h` right now, but really doesn't seem like the
right header to hold this specialization. Perhaps we should consider adding
something like an `AST/ForwardDecls.h`, containing forward declarations and
specializations such as this one, and include that from `Expr.h` and from here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87080/new/
https://reviews.llvm.org/D87080
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
