[clang] Implement CTAD for type alias template. #77890
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I'm thrilled to see work in this area. Thanks! Note that Clang 18 is going to branch soon, so this PR is going to target Clang 19. I do believe this is the last unimplemented features for our C++20 support (outside of bugs and modules) |
cor3ntin
added
c++20
clang:frontend
hokein
force-pushed
the
ctad-alias-template-draft
branch
3 times, most recently
from
a4b028d to
0f2d4fe
Compare
Yes, this patch will not be ready before the 18 release cut, so targeting clang 19 sounds like a good plan. As you may notice, the implementation is incomplete, and is likely far from perfect, I can see two alternatives to proceed:
Re 1), it is hard to predict the timeline, I'd like to finish it soon, but I also have other tasks with higher priority. Any thoughts? @cor3ntin, @shafik, @AaronBallman |
hokein
force-pushed
the
ctad-alias-template-draft
branch
from
0f2d4fe to
8f7d83a
Compare
hokein
changed the title
hokein
force-pushed
the
ctad-alias-template-draft
branch
from
8f7d83a to
becb1bd
Compare
|
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesFixes #54051 This patch implements the C++20 feature -- CTAD for alias templates. This patch also refactors the existing CC @ilya-biryukov, @sam-mccall , @usx95 Patch is 52.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77890.diff 10 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b5946e3f3178ff..c3fba8add4b660 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9305,6 +9305,12 @@ class Sema final {
const TemplateArgumentList &TemplateArgs,
sema::TemplateDeductionInfo &Info);
+ TemplateDeductionResult DeduceTemplateArguments(
+ TemplateParameterList *TemplateParams, ArrayRef<TemplateArgument> Ps,
+ ArrayRef<TemplateArgument> As, sema::TemplateDeductionInfo &Info,
+ SmallVectorImpl<DeducedTemplateArgument> &Deduced,
+ bool NumberOfArgumentsMustMatch);
+
TemplateDeductionResult SubstituteExplicitTemplateArguments(
FunctionTemplateDecl *FunctionTemplate,
TemplateArgumentListInfo &ExplicitTemplateArgs,
@@ -10457,9 +10463,11 @@ class Sema final {
SourceLocation PointOfInstantiation, FunctionDecl *Decl,
ArrayRef<TemplateArgument> TemplateArgs,
ConstraintSatisfaction &Satisfaction);
- FunctionDecl *InstantiateFunctionDeclaration(FunctionTemplateDecl *FTD,
- const TemplateArgumentList *Args,
- SourceLocation Loc);
+ FunctionDecl *InstantiateFunctionDeclaration(
+ FunctionTemplateDecl *FTD, const TemplateArgumentList *Args,
+ SourceLocation Loc,
+ CodeSynthesisContext::SynthesisKind CSC =
+ CodeSynthesisContext::ExplicitTemplateArgumentSubstitution);
void InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
FunctionDecl *Function,
bool Recursive = false,
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 1856a88e9a3271..7a55406171ac74 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -14,6 +14,7 @@ clang_tablegen(OpenCLBuiltins.inc -gen-clang-opencl-builtins
add_clang_library(clangSema
AnalysisBasedWarnings.cpp
+ CTAD.cpp
CodeCompleteConsumer.cpp
DeclSpec.cpp
DelayedDiagnostic.cpp
diff --git a/clang/lib/Sema/CTAD.cpp b/clang/lib/Sema/CTAD.cpp
new file mode 100644
index 00000000000000..745878c717fc4c
--- /dev/null
+++ b/clang/lib/Sema/CTAD.cpp
@@ -0,0 +1,209 @@
+//===--- CTAD.cpp - -------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CTAD.h"
+#include "TreeTransform.h"
+#include "TypeLocBuilder.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
+#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
+#include <optional>
+
+namespace clang {
+
+namespace {
+/// Tree transform to "extract" a transformed type from a class template's
+/// constructor to a deduction guide.
+class ExtractTypeForDeductionGuide
+ : public TreeTransform<ExtractTypeForDeductionGuide> {
+ llvm::SmallVectorImpl<TypedefNameDecl *> &MaterializedTypedefs;
+
+public:
+ typedef TreeTransform<ExtractTypeForDeductionGuide> Base;
+ ExtractTypeForDeductionGuide(
+ Sema &SemaRef,
+ llvm::SmallVectorImpl<TypedefNameDecl *> &MaterializedTypedefs)
+ : Base(SemaRef), MaterializedTypedefs(MaterializedTypedefs) {}
+
+ TypeSourceInfo *transform(TypeSourceInfo *TSI) { return TransformType(TSI); }
+
+ QualType TransformTypedefType(TypeLocBuilder &TLB, TypedefTypeLoc TL) {
+ ASTContext &Context = SemaRef.getASTContext();
+ TypedefNameDecl *OrigDecl = TL.getTypedefNameDecl();
+ TypedefNameDecl *Decl = OrigDecl;
+ // Transform the underlying type of the typedef and clone the Decl only if
+ // the typedef has a dependent context.
+ if (OrigDecl->getDeclContext()->isDependentContext()) {
+ TypeLocBuilder InnerTLB;
+ QualType Transformed =
+ TransformType(InnerTLB, OrigDecl->getTypeSourceInfo()->getTypeLoc());
+ TypeSourceInfo *TSI = InnerTLB.getTypeSourceInfo(Context, Transformed);
+ if (isa<TypeAliasDecl>(OrigDecl))
+ Decl = TypeAliasDecl::Create(
+ Context, Context.getTranslationUnitDecl(), OrigDecl->getBeginLoc(),
+ OrigDecl->getLocation(), OrigDecl->getIdentifier(), TSI);
+ else {
+ assert(isa<TypedefDecl>(OrigDecl) && "Not a Type alias or typedef");
+ Decl = TypedefDecl::Create(
+ Context, Context.getTranslationUnitDecl(), OrigDecl->getBeginLoc(),
+ OrigDecl->getLocation(), OrigDecl->getIdentifier(), TSI);
+ }
+ MaterializedTypedefs.push_back(Decl);
+ }
+
+ QualType TDTy = Context.getTypedefType(Decl);
+ TypedefTypeLoc TypedefTL = TLB.push<TypedefTypeLoc>(TDTy);
+ TypedefTL.setNameLoc(TL.getNameLoc());
+
+ return TDTy;
+ }
+};
+} // namespace
+
+ParmVarDecl *transformFunctionTypeParam(
+ Sema &SemaRef, ParmVarDecl *OldParam, DeclContext *DC,
+ MultiLevelTemplateArgumentList &Args,
+ llvm::SmallVectorImpl<TypedefNameDecl *> &MaterializedTypedefs) {
+ TypeSourceInfo *OldDI = OldParam->getTypeSourceInfo();
+ TypeSourceInfo *NewDI;
+ if (auto PackTL = OldDI->getTypeLoc().getAs<PackExpansionTypeLoc>()) {
+ // Expand out the one and only element in each inner pack.
+ Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(SemaRef, 0);
+ NewDI = SemaRef.SubstType(PackTL.getPatternLoc(), Args,
+ OldParam->getLocation(), OldParam->getDeclName());
+ if (!NewDI)
+ return nullptr;
+ NewDI = SemaRef.CheckPackExpansion(NewDI, PackTL.getEllipsisLoc(),
+ PackTL.getTypePtr()->getNumExpansions());
+ } else
+ NewDI = SemaRef.SubstType(OldDI, Args, OldParam->getLocation(),
+ OldParam->getDeclName());
+ if (!NewDI)
+ return nullptr;
+
+ // Extract the type. This (for instance) replaces references to typedef
+ // members of the current instantiations with the definitions of those
+ // typedefs, avoiding triggering instantiation of the deduced type during
+ // deduction.
+ NewDI = ExtractTypeForDeductionGuide(SemaRef, MaterializedTypedefs)
+ .transform(NewDI);
+
+ // Resolving a wording defect, we also inherit default arguments from the
+ // constructor.
+ ExprResult NewDefArg;
+ if (OldParam->hasDefaultArg()) {
+ // We don't care what the value is (we won't use it); just create a
+ // placeholder to indicate there is a default argument.
+ QualType ParamTy = NewDI->getType();
+ NewDefArg = new (SemaRef.Context)
+ OpaqueValueExpr(OldParam->getDefaultArgRange().getBegin(),
+ ParamTy.getNonLValueExprType(SemaRef.Context),
+ ParamTy->isLValueReferenceType() ? VK_LValue
+ : ParamTy->isRValueReferenceType() ? VK_XValue
+ : VK_PRValue);
+ }
+ // Handle arrays and functions decay.
+ auto NewType = NewDI->getType();
+ if (NewType->isArrayType() || NewType->isFunctionType())
+ NewType = SemaRef.Context.getDecayedType(NewType);
+
+ ParmVarDecl *NewParam = ParmVarDecl::Create(
+ SemaRef.Context, DC, OldParam->getInnerLocStart(),
+ OldParam->getLocation(), OldParam->getIdentifier(), NewType, NewDI,
+ OldParam->getStorageClass(), NewDefArg.get());
+ NewParam->setScopeInfo(OldParam->getFunctionScopeDepth(),
+ OldParam->getFunctionScopeIndex());
+ SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldParam, NewParam);
+ return NewParam;
+}
+
+TemplateTypeParmDecl *
+transformTemplateTypeParam(Sema &SemaRef, DeclContext *DC,
+ TemplateTypeParmDecl *TTP,
+ MultiLevelTemplateArgumentList &Args,
+ unsigned NewDepth, unsigned NewIndex) {
+ // TemplateTypeParmDecl's index cannot be changed after creation, so
+ // substitute it directly.
+ auto *NewTTP = TemplateTypeParmDecl::Create(
+ SemaRef.Context, DC, TTP->getBeginLoc(), TTP->getLocation(), NewDepth,
+ NewIndex, TTP->getIdentifier(), TTP->wasDeclaredWithTypename(),
+ TTP->isParameterPack(), TTP->hasTypeConstraint(),
+ TTP->isExpandedParameterPack()
+ ? std::optional<unsigned>(TTP->getNumExpansionParameters())
+ : std::nullopt);
+ if (const auto *TC = TTP->getTypeConstraint())
+ SemaRef.SubstTypeConstraint(NewTTP, TC, Args,
+ /*EvaluateConstraint*/ true);
+ if (TTP->hasDefaultArgument()) {
+ TypeSourceInfo *InstantiatedDefaultArg =
+ SemaRef.SubstType(TTP->getDefaultArgumentInfo(), Args,
+ TTP->getDefaultArgumentLoc(), TTP->getDeclName());
+ if (InstantiatedDefaultArg)
+ NewTTP->setDefaultArgument(InstantiatedDefaultArg);
+ }
+ SemaRef.CurrentInstantiationScope->InstantiatedLocal(TTP, NewTTP);
+ return NewTTP;
+}
+
+FunctionTemplateDecl *
+buildDeductionGuide(Sema &SemaRef, TemplateDecl *OriginalTemplate,
+ TemplateParameterList *TemplateParams,
+ CXXConstructorDecl *Ctor, ExplicitSpecifier ES,
+ TypeSourceInfo *TInfo, SourceLocation LocStart,
+ SourceLocation Loc, SourceLocation LocEnd, bool IsImplicit,
+ llvm::ArrayRef<TypedefNameDecl *> MaterializedTypedefs) {
+ DeclContext *DC = OriginalTemplate->getDeclContext();
+ auto DeductionGuideName =
+ SemaRef.Context.DeclarationNames.getCXXDeductionGuideName(
+ OriginalTemplate);
+
+ DeclarationNameInfo Name(DeductionGuideName, Loc);
+ ArrayRef<ParmVarDecl *> Params =
+ TInfo->getTypeLoc().castAs<FunctionProtoTypeLoc>().getParams();
+
+ // Build the implicit deduction guide template.
+ auto *Guide =
+ CXXDeductionGuideDecl::Create(SemaRef.Context, DC, LocStart, ES, Name,
+ TInfo->getType(), TInfo, LocEnd, Ctor);
+ Guide->setImplicit(IsImplicit);
+ Guide->setParams(Params);
+
+ for (auto *Param : Params)
+ Param->setDeclContext(Guide);
+ for (auto *TD : MaterializedTypedefs)
+ TD->setDeclContext(Guide);
+
+ auto *GuideTemplate = FunctionTemplateDecl::Create(
+ SemaRef.Context, DC, Loc, DeductionGuideName, TemplateParams, Guide);
+ GuideTemplate->setImplicit(IsImplicit);
+ Guide->setDescribedFunctionTemplate(GuideTemplate);
+
+ if (isa<CXXRecordDecl>(DC)) {
+ Guide->setAccess(AS_public);
+ GuideTemplate->setAccess(AS_public);
+ }
+
+ DC->addDecl(GuideTemplate);
+ return GuideTemplate;
+}
+
+} // namespace clang
diff --git a/clang/lib/Sema/CTAD.h b/clang/lib/Sema/CTAD.h
new file mode 100644
index 00000000000000..88110230318f81
--- /dev/null
+++ b/clang/lib/Sema/CTAD.h
@@ -0,0 +1,65 @@
+//===--- CTAD.h - Helper functions for CTAD -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines helper functions for the class template argument deduction
+// (CTAD) implementation.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
+
+namespace clang {
+
+// Transform a given function parameter decl into a deduction guide parameter
+// decl.
+ParmVarDecl *transformFunctionTypeParam(
+ Sema &SemaRef, ParmVarDecl *OldParam, DeclContext *DC,
+ MultiLevelTemplateArgumentList &Args,
+ llvm::SmallVectorImpl<TypedefNameDecl *> &MaterializedTypedefs);
+
+// Transform a given template type parameter into a deduction guide template
+// parameter, rebuilding any internal references to earlier parameters and
+// re-indexing as we go.
+TemplateTypeParmDecl *transformTemplateTypeParam(
+ Sema &SemaRef, DeclContext *DC, TemplateTypeParmDecl *TPT,
+ MultiLevelTemplateArgumentList &Args, unsigned NewDepth, unsigned NewIndex);
+// Similar to above, but for non-type template or template template parameters.
+template <typename NonTypeTemplateOrTemplateTemplateParmDecl>
+NonTypeTemplateOrTemplateTemplateParmDecl *
+transformTemplateParam(Sema &SemaRef, DeclContext *DC,
+ NonTypeTemplateOrTemplateTemplateParmDecl *OldParam,
+ MultiLevelTemplateArgumentList &Args,
+ unsigned NewIndex) {
+ // Ask the template instantiator to do the heavy lifting for us, then adjust
+ // the index of the parameter once it's done.
+ auto *NewParam = cast<NonTypeTemplateOrTemplateTemplateParmDecl>(
+ SemaRef.SubstDecl(OldParam, DC, Args));
+ NewParam->setPosition(NewIndex);
+ return NewParam;
+}
+
+// Build a deduction guide with the specified parameter types.
+FunctionTemplateDecl *buildDeductionGuide(
+ Sema &SemaRef, TemplateDecl *OriginalTemplate,
+ TemplateParameterList *TemplateParams, CXXConstructorDecl *Ctor,
+ ExplicitSpecifier ES, TypeSourceInfo *TInfo, SourceLocation LocStart,
+ SourceLocation Loc, SourceLocation LocEnd, bool IsImplicit,
+ llvm::ArrayRef<TypedefNameDecl *> MaterializedTypedefs = {});
+
+} // namespace clang
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 457fa377355a97..e6105040af8111 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -10,13 +10,19 @@
//
//===----------------------------------------------------------------------===//
+#include "CTAD.h"
+#include "TypeLocBuilder.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclAccessPair.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
-#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
#include "clang/AST/ExprOpenMP.h"
#include "clang/AST/IgnoreExpr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/SourceManager.h"
@@ -28,7 +34,9 @@
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Ownership.h"
#include "clang/Sema/SemaInternal.h"
+#include "clang/Sema/Template.h"
#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/SmallString.h"
@@ -10583,6 +10591,213 @@ static bool isOrIsDerivedFromSpecializationOf(CXXRecordDecl *RD,
return !(NotSpecialization(RD) && RD->forallBases(NotSpecialization));
}
+// Transform to form a corresponding deduction guide for type alias template
+// decl.
+//
+// This class implements the C++ [over.match.class.deduct]p3:
+// ... Let g denote the result of substituting these deductions into f. If
+// substitution succeeds, form a function or function template f' with the
+// following properties and add it to the set of guides of A...
+class AliasTemplateDeductionGuideTransform {
+public:
+ AliasTemplateDeductionGuideTransform(Sema &S, TypeAliasTemplateDecl *Alias)
+ : SemaRef(S), AliasTemplate(Alias), DC(Alias->getDeclContext()) {}
+ // Returns the result of substituting the deduced template arguments into F.
+ NamedDecl *transform(CXXDeductionGuideDecl *F,
+ ArrayRef<TemplateArgument> DeducedArgs,
+ ArrayRef<NamedDecl *> NonDeducedTemplateParamsInF) {
+ // Template parameters of the f'.
+ //
+ // C++ [over.match.class.deduct]p3.2:
+ // If f is a function template, f' is a function template whose template
+ // parameter list consists of all the template parameters of A (including
+ // their default template arguments) that appear in the above deductions
+ // or (recursively) in their default template arguments
+ SmallVector<NamedDecl *> TemplateParamsInFPrime =
+ FindAppearedTemplateParamsInAlias(DeducedArgs);
+ // ...followed by the template parameters of f that were not deduced
+ // (including their default template arguments)
+ TemplateParamsInFPrime.append(NonDeducedTemplateParamsInF.begin(),
+ NonDeducedTemplateParamsInF.end());
+
+ LocalInstantiationScope Scope(SemaRef);
+ SmallVector<TemplateArgument, 16> Depth1Args;
+ SmallVector<NamedDecl *, 16> AllParams;
+ SmallVector<TemplateArgument, 16> SubstArgs;
+ unsigned TemplateParamIndex = 0;
+ TemplateParameterList *TemplateParams = nullptr;
+
+ for (NamedDecl *Param : TemplateParamsInFPrime) {
+ MultiLevelTemplateArgumentList Args;
+
+ Args.setKind(TemplateSubstitutionKind::Rewrite);
+ Args.addOuterTemplateArguments(Depth1Args);
+ Args.addOuterRetainedLevel();
+ NamedDecl *NewParam =
+ transformTemplateParameter(Param, Args, TemplateParamIndex++);
+ if (!NewParam) {
+ llvm::errs() << "Faile to generate new param!\n";
+ return nullptr;
+ }
+ auto NewArgumentForNewParam =
+ SemaRef.Context.getCanonicalTemplateArgument(
+ SemaRef.Context.getInjectedTemplateArg(NewParam));
+ Depth1Args.push_back(NewArgumentForNewParam);
+ AllParams.push_back(NewParam);
+ SubstArgs.push_back(NewArgumentForNewParam);
+ }
+ // FIXME: substitute new template parameters into the requires-clause.
+ TemplateParams = TemplateParameterList::Create(
+ SemaRef.Context,
+ AliasTemplate->getTemplateParameters()->getTemplateLoc(),
+ AliasTemplate->getTemplateParameters()->getLAngleLoc(), AllParams,
+ AliasTemplate->getTemplateParameters()->getRAngleLoc(),
+ /*RequiresClause=*/nullptr);
+
+ MultiLevelTemplateArgumentList Args;
+ Args.setKind(TemplateSubstitutionKind::Rewrite);
+ Args.addOuterTemplateArguments(SubstArgs);
+ Args.addOuterRetainedLevel();
+
+ FunctionProtoTypeLoc FPTL = F->getTypeSourceInfo()
+ ->getTypeLoc()
+ .getAsAdjusted<FunctionProtoTypeLoc>();
+ assert(FPTL && "no prototype for underlying deduction guides");
+
+ // Transform the type of the function, adjusting the return type and
+ // replacing references to the old parameters with references to the
+ // new ones.
+ TypeLocBuilder TLB;
+ SmallVector<ParmVarDecl *, 8> Params;
+ SmallVector<TypedefNameDecl *, 4> MaterializedTypedefs;
+ QualType NewType = transformFunctionProtoType(
+ TLB, FPTL, Params, Args, F->getReturnType(), MaterializedTypedefs);
+ if (NewType.isNull())
+ return nullptr;
+ TypeSourceInfo *NewTInfo = TLB.getTypeSourceInfo(SemaRef.Context, NewType);
+
+ return clang::buildDeductionGuide(
+ SemaRef, AliasTemplate, TemplateParams,
+ F->getCorrespondingConstructor(), F->getExplicitSpecifier(), NewTInfo,
+ AliasTemplate->getBeginLoc(), AliasTemplate->getLocation(),
+ AliasTemplate->getEndLoc(), F->isImplicit(), MaterializedTypedefs);
+ }
+
+private:
+ // Find all template parameters of the AliasTemplate that appear in the
+ // DeducedArgs.
+ SmallVector<NamedDecl *>
+ FindAppearedTemplateParamsInAlias(ArrayRef<TemplateArgument> DeducedArgs) {
+ struct FindAppearedTemplateParams
+ : public RecursiveASTVisitor<FindAppearedTemplateParams> {
+ llvm::DenseSet<NamedDecl *> TemplateParamsInAlias...
[truncated]
|
|
@hokein @sam-mccall to repeat what was said in the language group last week. We do not think a feature flag is a good fit.
And if we break trunk, we revert, you get extra test cases... it's perfectly fine
We do that for about every feature that is not concept/module scale (and then again, these things were tses), and this isn't big enough to warrant a novel approach. I still think we should land this soon and we can start a review whenever you are ready. Does that make sense to you? |
|
Thanks for the summary.
I think this works fine with clang open-source release cycles (18 release was just cut, we have 6 months from now to polish this feature). However, this is not applicable for our internal case. At Google, we have used C++20 in production, and we have our own toolchain release cycles (which stay close to upstream main). Checking in an in-development C++20 feature without a proper safeguard poses risks to users (clang crashes, incorrect compilations, and other issues). So having a temporary flag is useful, it enables us to incrementally test and experiment it without compromising the experience for most users.
We could revert the whole patch when things are broken (and it is what we usually do). But I'm not sure revert/reland a big patch back and forth is a good idea. This also brings a general question: what level of quality should we require from experimental features in HEAD?
Yeah, I agree that this is a narrow feature. Given that this feature is very late to the party, and C++20 has been used in some productions, I think it is probably reasonable to have a flag.
+1, I think the current state should be good enough and ready for review. The remaining big piece (see the FIXME) is to implement the associated constraints (over.match.class.deduct#3.3). |
|
I started a discussion on discourse: https://discourse.llvm.org/t/a-temporary-flag-for-guarding-access-to-an-in-development-c-20-feature-in-clang/76597. |
There was a problem hiding this comment.
In order to aid the discussion on the temporary flag, I think it would be helpful to have a full set of tests that show what elements won't work right with the current set of changes.
I think this will help us gauge how impactful partial support would be and maybe help guide the discussion on alternatives.
This is effectively what we did repeatedly with the deferred concepts patch and IMO, was a really nice healthy way of doing it. If you organize your commits (much easier on github than it was on Phab), you can present JUST the changes between revisions/fixes and get a really nice revert/fix/release cycle going. I'd encourage just enabling by default (to avoid losing the test breadth that comes along with it), and reverting if there are sufficiently 'bad' breakages. Note that you'd not have to revert for all 'bugs', just those that break existing code. We can hold off on setting the Feature Test Macro until we're comfortable with it though. |
clang/lib/Sema/SemaInit.cpp
Outdated
| @@ -10636,6 +10889,113 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer( | |||
| // clear on this, but they're not found by name so access does not apply. | |||
| Guides.suppressDiagnostics(); | |||
|
|
|||
| SmallVector<DeclAccessPair> GuidesCandidates; | |||
| if (AliasTemplate) { | |||
There was a problem hiding this comment.
I was wondering if we need to separate the code that tweaks the candidate set into another function.
There was a problem hiding this comment.
Yes, and this is not the proper place to construct deduction guides for alias templates (they should live in DeclareImplicitDeductionGuides).
hokein
force-pushed
the
ctad-alias-template-draft
branch
from
becb1bd to
05740ec
Compare
|
This patch is ready for review now. |
clang/include/clang/Sema/Sema.h
Outdated
| CodeSynthesisContext::SynthesisKind CSC = | ||
| CodeSynthesisContext::ExplicitTemplateArgumentSubstitution); |
There was a problem hiding this comment.
Can you explain the default value here? If there was a default at all, one would think it would be TemplateInstantiation
There was a problem hiding this comment.
This is a refactoring and non-functional change, before this patch, we always use the CodeSynthesisContext::ExplicitTemplateArgumentSubstitution in this API implementation, the change here is to promote it to a function parameter (making a default value so that we don't need to change all existing callsites)
clang/lib/Sema/SemaTemplate.cpp
Outdated
| F->getTemplateParameters()->size()); | ||
| // FIXME: DeduceTemplateArguments stops immediately at the first | ||
| // non-deducible template parameter, extend it to continue performing | ||
| // deduction for rest of parameters. |
There was a problem hiding this comment.
You have a test case for that?
There was a problem hiding this comment.
This doesn't seem to cause any issues after trying a bunch cases.
The major testcase in my mind was using AFoo = Foo<double, int> with Foo<IsInt X, typename Y>, DeduceTemplateArguments will stop at the first argument -- we can't deduce X from double as double doesn't satisfy the IsInt constraint. But my understanding was incorrect, DeduceTemplateArguments still deduce the template arguments successfully for this case (X -> double, Y -> int). The concept constraint check is performed in other places (added this case in the unittest).
the other one is the case where template argument type mismatches template parameter type, e.g. template<typename T> class Foo; template<int N> using AFoo = Foo<N>;, this has been diagnosed when parsing the using alias.
I think we're fine here (and the FIXME is still valid).
| if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(TemplateParam)) | ||
| return transformTemplateTypeParam(SemaRef, DC, TTP, Args, | ||
| TTP->getDepth(), NewIndex); | ||
| if (auto *TTP = dyn_cast<TemplateTemplateParmDecl>(TemplateParam)) | ||
| return transformTemplateParam(SemaRef, DC, TTP, Args, NewIndex, | ||
| TTP->getDepth()); | ||
| if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(TemplateParam)) | ||
| return transformTemplateParam(SemaRef, DC, NTTP, Args, NewIndex, | ||
| NTTP->getDepth()); |
There was a problem hiding this comment.
Can we put that in a function?
There was a problem hiding this comment.
This lambda is only used inside this function (called twice below), I think it is better to keep it local (there is a similar variant transformTemplateParameter function in this file as well, I'd avoid adding one more)
hokein
force-pushed
the
ctad-alias-template-draft
branch
from
caf055b to
59e1263
Compare
clang/include/clang/Sema/Sema.h
Outdated
| CodeSynthesisContext::SynthesisKind CSC = | ||
| CodeSynthesisContext::ExplicitTemplateArgumentSubstitution); |
There was a problem hiding this comment.
This is a refactoring and non-functional change, before this patch, we always use the CodeSynthesisContext::ExplicitTemplateArgumentSubstitution in this API implementation, the change here is to promote it to a function parameter (making a default value so that we don't need to change all existing callsites)
clang/lib/Sema/SemaTemplate.cpp
Outdated
| // Find all template parameters of the AliasTemplate that appear in the | ||
| // given DeducedArgs. | ||
| SmallVector<unsigned> | ||
| FindAppearedTemplateParamsInAlias(ArrayRef<TemplateArgument> DeducedArgs, |
There was a problem hiding this comment.
I'm leaning towards Appeared to align with the C++ standard. It basically does "whose template parameter list consists of all the template parameters of A (including their default template arguments) that appear in the above deductions"
clang/lib/Sema/SemaTemplate.cpp
Outdated
| F->getTemplateParameters()->size()); | ||
| // FIXME: DeduceTemplateArguments stops immediately at the first | ||
| // non-deducible template parameter, extend it to continue performing | ||
| // deduction for rest of parameters. |
There was a problem hiding this comment.
This doesn't seem to cause any issues after trying a bunch cases.
The major testcase in my mind was using AFoo = Foo<double, int> with Foo<IsInt X, typename Y>, DeduceTemplateArguments will stop at the first argument -- we can't deduce X from double as double doesn't satisfy the IsInt constraint. But my understanding was incorrect, DeduceTemplateArguments still deduce the template arguments successfully for this case (X -> double, Y -> int). The concept constraint check is performed in other places (added this case in the unittest).
the other one is the case where template argument type mismatches template parameter type, e.g. template<typename T> class Foo; template<int N> using AFoo = Foo<N>;, this has been diagnosed when parsing the using alias.
I think we're fine here (and the FIXME is still valid).
…lias templates P1814R0. This patch implements the C++20 feature -- CTAD for alias templates. This is an initial patch, which covers most of pieces, the major missing piece is to implement the associated constraints (over.match.class.deduct#3.3) for the synthesized deduction guides (we can address in a followup). This patch also refactors the existing `ConvertConstructorToDeductionGuideTransform` to allow code reuse.
- Move the implementation to SemaTemplate.cpp where the typical CTAD implementation lives, thus no sepatate CTAD.h/.cpp; - Rewrite the implementation, we leverage more on clang's template instantatiation mechanism to build the deduction guide, it implementation is simplier; - Some enhancements on TreeTransform and TemplateInstantiator to allow running on BuildingDeductionGuides mode; - Added more tests;
- simplify the if - add a release note
- add more tests
…nTemplateArgumentList
|
Can you mark the status of the paper in cxx_status.html as partial (with a Thanks |
- update err_deduced_non_class_template_specialization_type to include alias templates - update cxx_status.html - fix a typo in ReleaseNotes
hokein
force-pushed
the
ctad-alias-template-draft
branch
from
dc15d1f to
57c5b98
Compare
|
Thank you for all the reviews, I will wait 1 or 2 days before merging it in case anyone has more comments. |
Sure! |
Yeah, I plan to complete this feature before the next clang release. If anyone is willing to help, that would be even greater. BTW, should we close #54051 with this PR? or should we keep it open until the feature is fully implemented? The |
Fixes #54051
This patch implements the C++20 feature -- CTAD for alias templates (P1814R0, specified in https://eel.is/c++draft/over.match.class.deduct#3). It is an initial patch:
BuildingDeductionGuidesmode;