shafik added a comment.
I made mostly minor comments, the diff is difficult to follow wrt to what has
really changed but it mostly makes sense. I will probably need a second look.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:508-510
MLTAL = getTemplateInstantiationArgs(FD, nullptr, /*RelativeToPrimary*/ true,
/*Pattern*/ nullptr,
+ /*ForConstraintInstantiation*/ true);
----------------
================
Comment at: clang/lib/Sema/SemaConcept.cpp:584-586
ND, nullptr, /*RelativeToPrimary*/ true,
/*Pattern*/ nullptr,
+ /*ForConstraintInstantiation*/ true);
----------------
================
Comment at: clang/lib/Sema/SemaConcept.cpp:1065-1067
/*RelativeToPrimary*/ true,
/*Pattern*/ nullptr,
+ /*ForConstraintInstantiation*/ true);
----------------
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:6106-6108
Template, &StackTemplateArgs, /*RelativeToPrimary*/ true,
/*Pattern*/ nullptr,
+ /*ForConceptInstantiation*/ true);
----------------
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2879-2881
Template, /*InnerMost*/ NeedsReplacement ? nullptr : &DeducedTAL,
/*RelativeToPrimary*/ true, /*Pattern*/
+ nullptr, /*ForConstraintInstantiation*/ true);
----------------
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:246
///
-/// \param LookBeyondLambda Indicates that this collection of arguments should
-/// continue looking when it encounters a lambda generic call operator.
-///
-/// \param IncludeContainingStructArgs Indicates that this collection of
-/// arguments should include arguments for any class template that this
-/// declaration is included inside of.
+/// \param ForConstraintInstantiation Indicates that the collection of
arguments
+/// should continue looking when encountering a lambda generic call operator,
----------------
This read a little awkward.
Maybe something more like "When collecting arguments ForContraintInstantion
indicates that we should..."
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:255
+ bool ForConstraintInstantiation) {
+ // bool IncludeContainingStructArgs) {
// Accumulate the set of template argument lists in this structure.
----------------
Dead code?
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:262
- const auto *Ctx = dyn_cast<DeclContext>(D);
- if (!Ctx) {
- Ctx = D->getDeclContext();
-
- // Add template arguments from a variable template instantiation. For a
- // class-scope explicit specialization, there are no template arguments
- // at this level, but there may be enclosing template arguments.
- const auto *Spec = dyn_cast<VarTemplateSpecializationDecl>(D);
- if (Spec && !Spec->isClassScopeExplicitSpecialization()) {
- // We're done when we hit an explicit specialization.
- if (Spec->getSpecializationKind() == TSK_ExplicitSpecialization &&
- !isa<VarTemplatePartialSpecializationDecl>(Spec))
- return Result;
-
- Result.addOuterTemplateArguments(&Spec->getTemplateInstantiationArgs());
-
- // If this variable template specialization was instantiated from a
- // specialized member that is a variable template, we're done.
- assert(Spec->getSpecializedTemplate() && "No variable template?");
- llvm::PointerUnion<VarTemplateDecl*,
- VarTemplatePartialSpecializationDecl*> Specialized
- = Spec->getSpecializedTemplateOrPartial();
- if (VarTemplatePartialSpecializationDecl *Partial =
- Specialized.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
- if (Partial->isMemberSpecialization())
- return Result;
- } else {
- VarTemplateDecl *Tmpl = Specialized.get<VarTemplateDecl *>();
- if (Tmpl->isMemberSpecialization())
- return Result;
- }
- }
-
- // If we have a template template parameter with translation unit context,
- // then we're performing substitution into a default template argument of
- // this template template parameter before we've constructed the template
- // that will own this template template parameter. In this case, we
- // use empty template parameter lists for all of the outer templates
- // to avoid performing any substitutions.
- if (Ctx->isTranslationUnit()) {
- if (const auto *TTP = dyn_cast<TemplateTemplateParmDecl>(D)) {
- for (unsigned I = 0, N = TTP->getDepth() + 1; I != N; ++I)
- Result.addOuterTemplateArguments(None);
- return Result;
- }
- }
- }
-
- while (!Ctx->isFileContext()) {
- // Add template arguments from a class template instantiation.
- const auto *Spec = dyn_cast<ClassTemplateSpecializationDecl>(Ctx);
- if (Spec && !Spec->isClassScopeExplicitSpecialization()) {
- // We're done when we hit an explicit specialization.
- if (Spec->getSpecializationKind() == TSK_ExplicitSpecialization &&
- !isa<ClassTemplatePartialSpecializationDecl>(Spec))
- break;
-
- Result.addOuterTemplateArguments(&Spec->getTemplateInstantiationArgs());
-
- // If this class template specialization was instantiated from a
- // specialized member that is a class template, we're done.
- assert(Spec->getSpecializedTemplate() && "No class template?");
- if (Spec->getSpecializedTemplate()->isMemberSpecialization())
- break;
- }
- // Add template arguments from a function template specialization.
- else if (const auto *Function = dyn_cast<FunctionDecl>(Ctx)) {
- if (!RelativeToPrimary &&
- Function->getTemplateSpecializationKindForInstantiation() ==
- TSK_ExplicitSpecialization)
- break;
-
- if (!RelativeToPrimary && Function->getTemplateSpecializationKind() ==
- TSK_ExplicitSpecialization) {
- // This is an implicit instantiation of an explicit specialization. We
- // don't get any template arguments from this function but might get
- // some from an enclosing template.
- } else if (const TemplateArgumentList *TemplateArgs
- = Function->getTemplateSpecializationArgs()) {
- // Add the template arguments for this specialization.
- Result.addOuterTemplateArguments(TemplateArgs);
-
- // If this function was instantiated from a specialized member that is
- // a function template, we're done.
- assert(Function->getPrimaryTemplate() && "No function template?");
- if (Function->getPrimaryTemplate()->isMemberSpecialization())
- break;
-
- // If this function is a generic lambda specialization, we are done.
- if (!LookBeyondLambda &&
- isGenericLambdaCallOperatorOrStaticInvokerSpecialization(Function))
- break;
-
- } else if (Function->getDescribedFunctionTemplate()) {
- assert((IncludeContainingStructArgs ||
- Result.getNumSubstitutedLevels() == 0) &&
- "Outer template not instantiated?");
- }
-
- // If this is a friend declaration and it declares an entity at
- // namespace scope, take arguments from its lexical parent
- // instead of its semantic parent, unless of course the pattern we're
- // instantiating actually comes from the file's context!
- if (Function->getFriendObjectKind() &&
- Function->getNonTransparentDeclContext()->isFileContext() &&
- (!Pattern || !Pattern->getLexicalDeclContext()->isFileContext())) {
- Ctx = Function->getLexicalDeclContext();
- RelativeToPrimary = false;
- continue;
- }
- } else if (const auto *Rec = dyn_cast<CXXRecordDecl>(Ctx)) {
- if (ClassTemplateDecl *ClassTemplate = Rec->getDescribedClassTemplate())
{
- assert((IncludeContainingStructArgs ||
- Result.getNumSubstitutedLevels() == 0) &&
- "Outer template not instantiated?");
- if (ClassTemplate->isMemberSpecialization())
- break;
- if (IncludeContainingStructArgs) {
- QualType RecordType = Context.getTypeDeclType(Rec);
- QualType Injected = cast<InjectedClassNameType>(RecordType)
- ->getInjectedSpecializationType();
- const auto *InjectedType =
cast<TemplateSpecializationType>(Injected);
- Result.addOuterTemplateArguments(InjectedType->template_arguments());
+ const Decl *CurDecl = ND;
+
----------------
Should we `assert` on `ND`?
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:290
+ return Result;
+ if (R.ChangeRelativeToPrimary)
+ RelativeToPrimary = false;
----------------
This reads weird, my instant is that this should flip to the opposite value
when this is `true`. Maybe an enum with names like `RelativeToPrimayOff` or
something more explicit like that.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:48
+ const Decl *NextDecl = nullptr;
+ bool ChangeRelativeToPrimary = true;
+ static Response Done() {
----------------
Maybe put both `bool` fields back to back.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134874/new/
https://reviews.llvm.org/D134874
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
