davrec added a subscriber: sammccall.
davrec added a comment.
My concerns have been addressed, except for some more nitpicking on names which
I think are uncontroversial.
Regarding the modules breakage found by @ChuanqiXu:
> Well we touch FunctionTemplates and VariableTemplates in this patch, because
> they were not doing template first.
> For whatever reason, class templates were already doing template first, so no
> need to fix that.
>
> So this patch at least puts that into consistency.
Is there already an analogous breakage for classes then?
Given @mizvekov's approaching deadline and @ChuanqiXu's limited availability,
I'm inclined to think if @mizvekov can hack together a reasonable fix for this
breakage, leaving a more rigorous/general solution for later as @ChuanqiXu
suggests he would like to pursue, that would be sufficient to land this. Does
anyone disagree?
================
Comment at: clang/include/clang/AST/ASTContext.h:1621-1623
+ Decl *ReplacedDecl, unsigned Index,
Optional<unsigned> PackIndex) const;
+ QualType getSubstTemplateTypeParmPackType(Decl *ReplacedDecl, unsigned Index,
----------------
AssociatedDecl
================
Comment at: clang/include/clang/AST/Type.h:5060
+ /// Gets the template parameter declaration that was substituted for.
+ const TemplateTypeParmDecl *getReplacedParameter() const;
+
----------------
@sammccall , drafting you as a representative for downstream AST users, do you
have any objection to changing this return type to a `TemplateTypeParmDecl`
from a `TemplateTypeParmType`? IIUC the change is not strictly necessary but
is more for convenience, correct me if I'm wrong @mizvekov. But I'm inclined
to think STTPT is not much used downstream so the churn will be
minimal/acceptable, am I wrong?
================
Comment at: clang/include/clang/AST/TypeProperties.td:730
}
+ def : Property<"replacedDecl", DeclRef> {
+ let Read = [{ node->getAssociatedDecl() }];
----------------
"associatedDecl"
================
Comment at: clang/include/clang/AST/TypeProperties.td:743
return ctx.getSubstTemplateTypeParmType(
- cast<TemplateTypeParmType>(replacedParameter),
- replacementType, PackIndex);
+ replacementType, replacedDecl, Index, PackIndex);
}]>;
----------------
associatedDecl
================
Comment at: clang/include/clang/AST/TypeProperties.td:762
let Class = SubstTemplateTypeParmPackType in {
- def : Property<"replacedParameter", QualType> {
- let Read = [{ QualType(node->getReplacedParameter(), 0) }];
+ def : Property<"replacedDecl", DeclRef> {
+ let Read = [{ node->getAssociatedDecl() }];
----------------
"associatedDecl"
================
Comment at: clang/include/clang/AST/TypeProperties.td:774
return ctx.getSubstTemplateTypeParmPackType(
- cast<TemplateTypeParmType>(replacedParameter),
- replacementPack);
+ replacedDecl, Index, replacementPack);
}]>;
----------------
associatedDecl
================
Comment at: clang/include/clang/Sema/Template.h:80
+ struct ArgumentListLevel {
+ Decl *AssociatedDecl;
+ ArgList Args;
----------------
Actually I think this one should be changed back to `ReplacedDecl` :)
ReplacedDecl makes perfect sense in MLTAL, AssociatedDecl seems to make better
sense in STTPT.
================
Comment at: clang/include/clang/Sema/Template.h:163
+ /// cases might even be a template parameter itself.
+ Decl *getAssociatedDecl(unsigned Depth) const {
+ assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels());
----------------
getReplacedDecl
================
Comment at: clang/include/clang/Sema/Template.h:205
/// list.
- void addOuterTemplateArguments(const TemplateArgumentList *TemplateArgs) {
- addOuterTemplateArguments(ArgList(TemplateArgs->data(),
- TemplateArgs->size()));
+ void addOuterTemplateArguments(Decl *AssociatedDecl, ArgList Args) {
+ assert(!NumRetainedOuterLevels &&
----------------
ReplacedDecl
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131858/new/
https://reviews.llvm.org/D131858
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
