[clang] CTAD: implement the missing IsDeducible constraint for alias templates #89358
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesThis patch implements the "IsDeducible" constraint where the template arguments of the alias template can be deduced from the returned type of the synthesized deduction guide, per C++ [over.match.class.deduct]p4. In the implementation, we perform the deduction directly, which is more efficient than the way specified in the standard. In this patch, we add a Also update relevant CTAD tests which were incorrectly compiled due to the missing constraint. Full diff: https://github.com/llvm/llvm-project/pull/89358.diff 9 Files Affected:
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index a27fbed358a60c..74102f40539681 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -537,6 +537,7 @@ TYPE_TRAIT_1(__is_referenceable, IsReferenceable, KEYCXX)
TYPE_TRAIT_1(__can_pass_in_regs, CanPassInRegs, KEYCXX)
TYPE_TRAIT_2(__reference_binds_to_temporary, ReferenceBindsToTemporary, KEYCXX)
TYPE_TRAIT_2(__reference_constructs_from_temporary, ReferenceConstructsFromTemporary, KEYCXX)
+TYPE_TRAIT_2(__is_deducible, IsDeducible, KEYCXX)
// Embarcadero Expression Traits
EXPRESSION_TRAIT(__is_lvalue_expr, IsLValueExpr, KEYCXX)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1e89dfc58d92b1..0d8477cf49aaf0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9591,6 +9591,15 @@ class Sema final : public SemaBase {
ArrayRef<TemplateArgument> TemplateArgs,
sema::TemplateDeductionInfo &Info);
+ /// Deduce the template arguments of the given template from \p FromType.
+ /// Used to implement the IsDeducible constraint for alias CTAD per C++
+ /// [over.match.class.deduct]p4.
+ ///
+ /// It only supports class or type alias templates.
+ TemplateDeductionResult
+ DeduceTemplateArgumentsFromType(TemplateDecl *TD, QualType FromType,
+ sema::TemplateDeductionInfo &Info);
+
TemplateDeductionResult DeduceTemplateArguments(
TemplateParameterList *TemplateParams, ArrayRef<TemplateArgument> Ps,
ArrayRef<TemplateArgument> As, sema::TemplateDeductionInfo &Info,
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 0d2ad980696fcc..af4e205eeff803 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3906,14 +3906,18 @@ ExprResult Parser::ParseTypeTrait() {
BalancedDelimiterTracker Parens(*this, tok::l_paren);
if (Parens.expectAndConsume())
return ExprError();
-
+ TypeTrait TTKind = TypeTraitFromTokKind(Kind);
SmallVector<ParsedType, 2> Args;
do {
// Parse the next type.
- TypeResult Ty = ParseTypeName(/*SourceRange=*/nullptr,
- getLangOpts().CPlusPlus
- ? DeclaratorContext::TemplateTypeArg
- : DeclaratorContext::TypeName);
+ TypeResult Ty = ParseTypeName(
+ /*SourceRange=*/nullptr,
+ getLangOpts().CPlusPlus
+ // For __is_deducible type trait, the first argument is a template
+ // specification type without template argument lists.
+ ? (TTKind == BTT_IsDeducible ? DeclaratorContext::TemplateArg
+ : DeclaratorContext::TemplateTypeArg)
+ : DeclaratorContext::TypeName);
if (Ty.isInvalid()) {
Parens.skipToEnd();
return ExprError();
@@ -3937,7 +3941,7 @@ ExprResult Parser::ParseTypeTrait() {
SourceLocation EndLoc = Parens.getCloseLocation();
- return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
+ return Actions.ActOnTypeTrait(TTKind, Loc, Args, EndLoc);
}
/// ParseArrayTypeTrait - Parse the built-in array type-trait
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 7582cbd75fec05..0833a985b48b88 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -6100,6 +6100,17 @@ static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT, const TypeSourceI
tok::kw___is_pointer_interconvertible_base_of);
return Self.IsPointerInterconvertibleBaseOf(Lhs, Rhs);
+ }
+ case BTT_IsDeducible: {
+ if (const auto *TSTToBeDeduced =
+ LhsT->getAs<DeducedTemplateSpecializationType>()) {
+ sema::TemplateDeductionInfo Info(KeyLoc);
+ return Self.DeduceTemplateArgumentsFromType(
+ TSTToBeDeduced->getTemplateName().getAsTemplateDecl(), RhsT,
+ Info) == TemplateDeductionResult::Success;
+ }
+ // FIXME: emit a diagnostic.
+ return false;
}
default: llvm_unreachable("not a BTT");
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index d4976f9d0d11d8..12a4ac05835557 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/TemplateName.h"
+#include "clang/AST/Type.h"
#include "clang/AST/TypeVisitor.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/DiagnosticSema.h"
@@ -2780,6 +2781,41 @@ Expr *transformRequireClause(Sema &SemaRef, FunctionTemplateDecl *FTD,
return E.getAs<Expr>();
}
+// Build the associated constraints for the alias deduction guides.
+// C++ [over.match.class.deduct]p3.3:
+// The associated constraints ([temp.constr.decl]) are the conjunction of the
+// associated constraints of g and a constraint that is satisfied if and only
+// if the arguments of A are deducible (see below) from the return type.
+Expr *
+buildAssociatedConstraints(Sema &SemaRef, TypeAliasTemplateDecl *AliasTemplate,
+ FunctionTemplateDecl *FTD,
+ llvm::ArrayRef<TemplateArgument> TransformedArgs,
+ QualType ReturnType) {
+ auto &Context = SemaRef.Context;
+ TemplateName TN(AliasTemplate);
+ auto TST = Context.getDeducedTemplateSpecializationType(TN, QualType(), true);
+ // Build the IsDeducible constraint.
+ SmallVector<TypeSourceInfo *> IsDeducibleTypeTraitArgs = {
+ Context.getTrivialTypeSourceInfo(TST),
+ Context.getTrivialTypeSourceInfo(ReturnType)};
+ Expr *IsDeducible = TypeTraitExpr::Create(
+ Context, Context.getLogicalOperationType(), AliasTemplate->getLocation(),
+ TypeTrait::BTT_IsDeducible, IsDeducibleTypeTraitArgs,
+ AliasTemplate->getLocation(), false);
+
+ // Substitute new template parameters into requires-clause if present.
+ if (auto *TransformedRC =
+ transformRequireClause(SemaRef, FTD, TransformedArgs)) {
+ auto Conjunction = SemaRef.BuildBinOp(
+ SemaRef.getCurScope(), SourceLocation{}, BinaryOperatorKind::BO_LAnd,
+ TransformedRC, IsDeducible);
+ if (Conjunction.isInvalid())
+ return nullptr;
+ return Conjunction.get();
+ }
+ return IsDeducible;
+}
+
std::pair<TemplateDecl *, llvm::ArrayRef<TemplateArgument>>
getRHSTemplateDeclAndArgs(Sema &SemaRef, TypeAliasTemplateDecl *AliasTemplate) {
// Unwrap the sugared ElaboratedType.
@@ -2962,19 +2998,6 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
Context.getCanonicalTemplateArgument(
Context.getInjectedTemplateArg(NewParam));
}
- // Substitute new template parameters into requires-clause if present.
- Expr *RequiresClause =
- transformRequireClause(SemaRef, F, TemplateArgsForBuildingFPrime);
- // FIXME: implement the is_deducible constraint per C++
- // [over.match.class.deduct]p3.3:
- // ... and a constraint that is satisfied if and only if the arguments
- // of A are deducible (see below) from the return type.
- auto *FPrimeTemplateParamList = TemplateParameterList::Create(
- Context, AliasTemplate->getTemplateParameters()->getTemplateLoc(),
- AliasTemplate->getTemplateParameters()->getLAngleLoc(),
- FPrimeTemplateParams,
- AliasTemplate->getTemplateParameters()->getRAngleLoc(),
- /*RequiresClause=*/RequiresClause);
// To form a deduction guide f' from f, we leverage clang's instantiation
// mechanism, we construct a template argument list where the template
@@ -3019,6 +3042,18 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
if (auto *FPrime = SemaRef.InstantiateFunctionDeclaration(
F, TemplateArgListForBuildingFPrime, AliasTemplate->getLocation(),
Sema::CodeSynthesisContext::BuildingDeductionGuides)) {
+ Expr *RequireClause = buildAssociatedConstraints(
+ SemaRef, AliasTemplate, F, TemplateArgsForBuildingFPrime,
+ FPrime->getReturnType());
+ if (!RequireClause)
+ continue;
+ auto *FPrimeTemplateParamList = TemplateParameterList::Create(
+ Context, AliasTemplate->getTemplateParameters()->getTemplateLoc(),
+ AliasTemplate->getTemplateParameters()->getLAngleLoc(),
+ FPrimeTemplateParams,
+ AliasTemplate->getTemplateParameters()->getRAngleLoc(),
+ RequireClause);
+
auto *GG = cast<CXXDeductionGuideDecl>(FPrime);
buildDeductionGuide(SemaRef, AliasTemplate, FPrimeTemplateParamList,
GG->getCorrespondingConstructor(),
@@ -3072,16 +3107,6 @@ FunctionTemplateDecl *DeclareAggregateDeductionGuideForTypeAlias(
SemaRef.Context.getInjectedTemplateArg(NewParam));
TransformedTemplateParams.push_back(NewParam);
}
- // FIXME: implement the is_deducible constraint per C++
- // [over.match.class.deduct]p3.3.
- Expr *TransformedRequiresClause = transformRequireClause(
- SemaRef, RHSDeductionGuide, TransformedTemplateArgs);
- auto *TransformedTemplateParameterList = TemplateParameterList::Create(
- SemaRef.Context, AliasTemplate->getTemplateParameters()->getTemplateLoc(),
- AliasTemplate->getTemplateParameters()->getLAngleLoc(),
- TransformedTemplateParams,
- AliasTemplate->getTemplateParameters()->getRAngleLoc(),
- TransformedRequiresClause);
auto *TransformedTemplateArgList = TemplateArgumentList::CreateCopy(
SemaRef.Context, TransformedTemplateArgs);
@@ -3089,6 +3114,18 @@ FunctionTemplateDecl *DeclareAggregateDeductionGuideForTypeAlias(
RHSDeductionGuide, TransformedTemplateArgList,
AliasTemplate->getLocation(),
Sema::CodeSynthesisContext::BuildingDeductionGuides)) {
+ Expr *RequireClause = buildAssociatedConstraints(
+ SemaRef, AliasTemplate, RHSDeductionGuide, TransformedTemplateArgs,
+ TransformedDeductionGuide->getReturnType());
+ if (!RequireClause)
+ return nullptr;
+ auto *TransformedTemplateParameterList = TemplateParameterList::Create(
+ SemaRef.Context,
+ AliasTemplate->getTemplateParameters()->getTemplateLoc(),
+ AliasTemplate->getTemplateParameters()->getLAngleLoc(),
+ TransformedTemplateParams,
+ AliasTemplate->getTemplateParameters()->getRAngleLoc(), RequireClause);
+
auto *GD =
llvm::dyn_cast<clang::CXXDeductionGuideDecl>(TransformedDeductionGuide);
FunctionTemplateDecl *Result = buildDeductionGuide(
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 0b6375001f5326..942c7343163e24 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3139,6 +3139,40 @@ static TemplateDeductionResult FinishTemplateArgumentDeduction(
return TemplateDeductionResult::Success;
}
+/// Complete template argument deduction for DeduceTemplateArgumentsFromType.
+/// FIXME: this is mostly duplicated with the above two versions. Deduplicate
+/// the three implementations.
+static TemplateDeductionResult FinishTemplateArgumentDeduction(
+ Sema &S, TemplateDecl *TD,
+ SmallVectorImpl<DeducedTemplateArgument> &Deduced,
+ TemplateDeductionInfo &Info) {
+ // Unevaluated SFINAE context.
+ EnterExpressionEvaluationContext Unevaluated(
+ S, Sema::ExpressionEvaluationContext::Unevaluated);
+ Sema::SFINAETrap Trap(S);
+
+ Sema::ContextRAII SavedContext(S, getAsDeclContextOrEnclosing(TD));
+
+ // C++ [temp.deduct.type]p2:
+ // [...] or if any template argument remains neither deduced nor
+ // explicitly specified, template argument deduction fails.
+ SmallVector<TemplateArgument, 4> SugaredBuilder, CanonicalBuilder;
+ if (auto Result = ConvertDeducedTemplateArguments(
+ S, TD, /*IsPartialOrdering=*/false, Deduced, Info, SugaredBuilder,
+ CanonicalBuilder);
+ Result != TemplateDeductionResult::Success)
+ return Result;
+
+ if (Trap.hasErrorOccurred())
+ return TemplateDeductionResult::SubstitutionFailure;
+
+ if (auto Result = CheckDeducedArgumentConstraints(S, TD, SugaredBuilder,
+ CanonicalBuilder, Info);
+ Result != TemplateDeductionResult::Success)
+ return Result;
+
+ return TemplateDeductionResult::Success;
+}
/// Perform template argument deduction to determine whether the given template
/// arguments match the given class or variable template partial specialization
@@ -3207,6 +3241,59 @@ Sema::DeduceTemplateArguments(VarTemplatePartialSpecializationDecl *Partial,
return ::DeduceTemplateArguments(*this, Partial, TemplateArgs, Info);
}
+TemplateDeductionResult
+Sema::DeduceTemplateArgumentsFromType(TemplateDecl *TD, QualType FromType,
+ sema::TemplateDeductionInfo &Info) {
+ if (TD->isInvalidDecl())
+ return TemplateDeductionResult::Invalid;
+
+ QualType PType;
+ if (const auto *CTD = dyn_cast<ClassTemplateDecl>(TD)) {
+ // Use the InjectedClassNameType.
+ PType = Context.getTypeDeclType(CTD->getTemplatedDecl());
+ } else if (const auto *AliasTemplate = dyn_cast<TypeAliasTemplateDecl>(TD)) {
+ PType = AliasTemplate->getTemplatedDecl()
+ ->getUnderlyingType()
+ .getCanonicalType();
+ } else {
+ // FIXME: emit a diagnostic, we only only support alias and class templates.
+ return TemplateDeductionResult::Invalid;
+ }
+
+ // Unevaluated SFINAE context.
+ EnterExpressionEvaluationContext Unevaluated(
+ *this, Sema::ExpressionEvaluationContext::Unevaluated);
+ SFINAETrap Trap(*this);
+
+ // This deduction has no relation to any outer instantiation we might be
+ // performing.
+ LocalInstantiationScope InstantiationScope(*this);
+
+ SmallVector<DeducedTemplateArgument> Deduced(
+ TD->getTemplateParameters()->size());
+ SmallVector<TemplateArgument> PArgs = {TemplateArgument(PType)};
+ SmallVector<TemplateArgument> AArgs = {TemplateArgument(FromType)};
+ if (auto DeducedResult = DeduceTemplateArguments(
+ TD->getTemplateParameters(), PArgs, AArgs, Info, Deduced, false);
+ DeducedResult != TemplateDeductionResult::Success) {
+ return DeducedResult;
+ }
+
+ SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(), Deduced.end());
+ InstantiatingTemplate Inst(*this, Info.getLocation(), TD, DeducedArgs, Info);
+ if (Inst.isInvalid())
+ return TemplateDeductionResult::InstantiationDepth;
+
+ if (Trap.hasErrorOccurred())
+ return TemplateDeductionResult::SubstitutionFailure;
+
+ TemplateDeductionResult Result;
+ runWithSufficientStackSpace(Info.getLocation(), [&] {
+ Result = ::FinishTemplateArgumentDeduction(*this, TD, Deduced, Info);
+ });
+ return Result;
+}
+
/// Determine whether the given type T is a simple-template-id type.
static bool isSimpleTemplateIdType(QualType T) {
if (const TemplateSpecializationType *Spec
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index 6f04264a655ad5..478f1d21c1865d 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -109,10 +109,12 @@ struct Foo {
};
template <typename X, int Y>
-using Bar = Foo<X, sizeof(X)>;
+using Bar = Foo<X, sizeof(X)>; // expected-note {{candidate template ignored: couldn't infer template argument 'X'}} \
+ // expected-note {{candidate template ignored: constraints not satisfied [with X = int]}} \
+ // expected-note {{because '__is_deducible(Bar, Foo<int, 4UL>)' evaluated to false}}
-// FIXME: we should reject this case? GCC rejects it, MSVC accepts it.
-Bar s = {{1}};
+
+Bar s = {{1}}; // expected-error {{no viable constructor or deduction guide }}
} // namespace test9
namespace test10 {
@@ -133,9 +135,13 @@ A a(2); // Foo<int*>
namespace test11 {
struct A {};
template<class T> struct Foo { T c; };
-template<class X, class Y=A> using AFoo = Foo<Y>;
+template<class X, class Y=A>
+using AFoo = Foo<Y>; // expected-note {{candidate template ignored: could not match 'Foo<type-parameter-0-0>' against 'int'}} \
+ // expected-note {{candidate template ignored: constraints not satisfied [with T = int]}} \
+ // expected-note {{because '__is_deducible(AFoo, Foo<int>)' evaluated to false}} \
+ // expected-note {{candidate function template not viable: requires 0 arguments, but 1 was provided}}
-AFoo s = {1};
+AFoo s = {1}; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'AFoo'}}
} // namespace test11
namespace test12 {
@@ -190,13 +196,15 @@ template <class T> struct Foo { Foo(T); };
template<class V> using AFoo = Foo<V *>;
template<typename> concept False = false;
-template<False W> using BFoo = AFoo<W>;
+template<False W>
+using BFoo = AFoo<W>; // expected-note {{candidate template ignored: constraints not satisfied [with V = int]}} \
+ // expected-note {{because '__is_deducible(BFoo, Foo<int *>)' evaluated to false}} \
+ // expected-note {{candidate template ignored: could not match 'Foo<type-parameter-0-0 *>' against 'int *'}}
int i = 0;
AFoo a1(&i); // OK, deduce Foo<int *>
-// FIXME: we should reject this case as the W is not deduced from the deduced
-// type Foo<int *>.
-BFoo b2(&i);
+// the W is not deduced from the deduced type Foo<int *>.
+BFoo b2(&i); // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'BFoo'}}
} // namespace test15
namespace test16 {
diff --git a/clang/test/SemaCXX/type-traits-is-deducible.cpp b/clang/test/SemaCXX/type-traits-is-deducible.cpp
new file mode 100644
index 00000000000000..1a9ba19fb6035d
--- /dev/null
+++ b/clang/test/SemaCXX/type-traits-is-deducible.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// expected-no-diagnostics
+
+template<typename T>
+struct Foo {};
+static_assert(__is_deducible(Foo, Foo<int>));
+static_assert(!__is_deducible(Foo, int));
+
+template <class T>
+using AFoo1 = Foo<T*>;
+static_assert(__is_deducible(AFoo1, Foo<int*>));
+static_assert(!__is_deducible(AFoo1, Foo<int>));
+
+template <class T>
+using AFoo2 = Foo<int>;
+static_assert(!__is_deducible(AFoo2, Foo<int>));
+
+// default template argument counts.
+template <class T = double>
+using AFoo3 = Foo<int>;
+static_assert(__is_deducible(AFoo3, Foo<int>));
+
+
+template <int N>
+struct Bar { int k = N; };
+static_assert(__is_deducible(Bar, Bar<1>));
+
+template <int N>
+using ABar1 = Bar<N>;
+static_assert(__is_deducible(ABar1, Bar<3>));
+template <int N>
+using ABar2 = Bar<1>;
+static_assert(!__is_deducible(ABar2, Bar<1>));
+
+
+template <typename T>
+class Forward;
+static_assert(__is_deducible(Forward, Forward<int>));
+template <typename T>
+using AForward = Forward<T>;
+static_assert(__is_deducible(AForward, Forward<int>));
+
+
+template <class T, T N>
+using AArrary = int[N];
+static_assert (__is_deducible(AArrary, int[42]));
+static_assert (!__is_deducible(AArrary, double[42]));
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index c233171e63c811..8b7c51a5610a90 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -868,13 +868,7 @@ <h2 id="cxx20">C++20 implementation status</h2>
<tr>
<td>Class template argument deduction for alias templates</td>
<td><a href="https://wg21.link/p1814r0">P1814R0</a></td>
- <td class="partial" align="center">
- <details>
- <summary>Clang 19 (Partial)</summary>
- The associated constraints (over.match.class.deduct#3.3) for the
- synthesized deduction guides are not yet implemented.
- </details>
- </td>
+ <td class="partial" align="center">Clang 19 (Partial)</td>
</tr>
<tr>
<td>Permit conversions to arrays of unknown bound</td>
|
|
Regarding the __is_deducible type trait, GCC also provides one, but it was hidden from users and only used for internal CTAD implementation. I'm not sure if we should follow the same strategy in clang, ideas? |
clang/lib/Sema/SemaExprCXX.cpp
Outdated
| TSTToBeDeduced->getTemplateName().getAsTemplateDecl(), RhsT, | ||
| Info) == TemplateDeductionResult::Success; | ||
| } | ||
| // FIXME: emit a diagnostic. |
There was a problem hiding this comment.
I dont think we can leave this as a fixme.
There was a problem hiding this comment.
implemented a new diagnostic for bad argument types for the builtin.
| ->getUnderlyingType() | ||
| .getCanonicalType(); | ||
| } else { | ||
| // FIXME: emit a diagnostic, we only only support alias and class templates. |
There was a problem hiding this comment.
It turns out we will never hit other cases in this code path (we will reject the var/function templates when parsing the type trait expr). I changed it to assert.
hokein
force-pushed
the
ctad-is-deducible
branch
from
19a50f5
to
dbf9d54
Compare
Added a release note for the new builtin. |
I have mixed feeling. What do you think @AaronBallman ? |
Personally, I do not like exposing type traits that aren't for use with the STL. One idea would be to remove the type trait from TokenKinds.def and instead manually add TYPE_TRAIT_2 that might need adjustment as well.) Then we can remove the release note, error checking can become assertions, etc.
|
clang/www/cxx_status.html
Outdated
| synthesized deduction guides are not yet implemented. | ||
| </details> | ||
| </td> | ||
| <td class="partial" align="center">Clang 19 (Partial)</td> |
There was a problem hiding this comment.
Why is this still partial?
There was a problem hiding this comment.
With this patch, this feature is completed. However, I suspect that there maybe issues, and it needs some time to be productized. Therefore, I hesitate to say that it is "ready" (although I'm not sure what the common practice is).
There was a problem hiding this comment.
Can you add back a details saying that the feature macro has not been updated?
However, i suspect that we do want to finalize the feature for Clang 19.
The argument types are not modeled as children of TypeTraitExpr, therefore they are not dumped with the default implementation. Dumping them is really useful for ad-hoc debugging, context #89358
This is a valid argument. This type trait is supposed to be clang-internal only and should not be used in other places. The main benefit of exposing it is for testing purposes, and I personally found it to be really useful for debugging.
This approach is doable technically, but it feels hacky and fragile. What if we emit an error (or warning) diagnostic and reject the code when we parse the |
Why do you think it is fragile? I think a comment around |
This is not a single-place change, we have to duplicate the
|
Something like that seem to be fine // IsDeducible is only used internally by clang and is not exposed to source code
TYPE_TRAIT_2(/**/, IsDeducible, KEYCXX) |
hokein
force-pushed
the
ctad-is-deducible
branch
2 times, most recently
from
5020665
to
c11f11b
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
OK, I have hidden the |
Sorry, I think my suggestion was not clear enough. |
Ah, I see. I haven't tried that, but one downside I can see is that it will make diagnostics for CTAD worse, we need to spell the type trait name in error messages when the |
I think we should have a custom diag for that anyway |
I think this is one of the symptoms, missing the trait name in the AST dump is another symptom. And I'm not aware of an easy way to customize a diagnostic within clang's template-constraint check mechanism. One implementation I can think of is to manual call the |
|
It appears that the consensus is to avoid exposing the internal Regarding implementation, the current tablegen-based mechanism doesn't offer an optimal solution for defining internal-only builtins. Several options:
Opinions and thoughts would be appreciated, @AaronBallman, @cor3ntin, @erichkeane |
|
@hokein Independently of the direction taken I'd like to see a better diagnostic than "atomic constraint using an undocumented/cryptic trait that is not in the code is not satisfied". So when we try to print atomic constraints, we should do something more user friendly for is_deducible. ( |
I agree with you -- having a well-described diagnostic message is better and clearer. I'm happy to improve it once we settle on the final implementation approach (the current diagnostic By the way, other parts of this patch are ready for review. |
Well, if we agree on that, the only thing to do for approach 3 is to deal with "anonymous" traits in ast dump and similar, which seems to be a fairly bounded effort! |
I was worried about the potential need to handle these issues in various places. However, thinking more about it today, I think we can address them locally in the I've now updated the patch to implement approach 3. Please take a look. Note that I haven't addressed the diagnostic improvement part yet; I plan to handle that in a separate follow-up patch. |
| @@ -27,7 +27,8 @@ enum TypeTrait { | |||
| , | |||
| #define TYPE_TRAIT_2(Spelling, Name, Key) BTT_##Name, | |||
| #include "clang/Basic/TokenKinds.def" | |||
| BTT_Last = UTT_Last // BTT_Last == last BTT_XX in the enum. | |||
| BTT_Last = UTT_Last | |||
There was a problem hiding this comment.
oops, this is not intended. This was left when I reverted some changes.
There was a problem hiding this comment.
Reverted the change.
| TemplateDeductionResult | ||
| Sema::DeduceTemplateArgumentsFromType(TemplateDecl *TD, QualType FromType, |
There was a problem hiding this comment.
We have a lot of DeduceTemplateArguments function so I think we want to add a comment here describing what that function does / when it is used.
There was a problem hiding this comment.
This function is public (the documentation and comments are in Sema.h header).
| /// Complete template argument deduction for DeduceTemplateArgumentsFromType. | ||
| /// FIXME: this is mostly duplicated with the above two versions. Deduplicate | ||
| /// the three implementations. |
There was a problem hiding this comment.
Is that something you would be willing to do in a follow up patch?
There was a problem hiding this comment.
It is in my TODO list, but its priority is currently low, so I don't anticipate addressing it in the near future. This seems like a good candidate for a "GoodFirst" issue, so I'm thinking we can file an issue in case someone wants to contribute.
clang/lib/Sema/SemaTemplate.cpp
Outdated
| FunctionTemplateDecl *FTD, | ||
| llvm::ArrayRef<TemplateArgument> TransformedArgs, | ||
| QualType ReturnType) { | ||
| auto &Context = SemaRef.Context; |
There was a problem hiding this comment.
| auto &Context = SemaRef.Context; | |
| (const?) ASTContext &Context = SemaRef.Context; |
There was a problem hiding this comment.
I'm not seeing a change here?
There was a problem hiding this comment.
Oops, it looks like I forgot to push one commit.
clang/lib/Sema/SemaTemplate.cpp
Outdated
| @@ -18,6 +18,7 @@ | |||
| #include "clang/AST/ExprCXX.h" | |||
| #include "clang/AST/RecursiveASTVisitor.h" | |||
| #include "clang/AST/TemplateName.h" | |||
| #include "clang/AST/Type.h" | |||
There was a problem hiding this comment.
Probably not a needed change
clang/lib/Sema/SemaExprCXX.cpp
Outdated
| TSTToBeDeduced->getTemplateName().getAsTemplateDecl(), RhsT, | ||
| Info) == TemplateDeductionResult::Success; | ||
| } | ||
| assert("Expect to see DeducedTemplateSpecializationType!"); |
There was a problem hiding this comment.
that is always true (you probably mean false && "").
But I would remove the if, use cast<DeducedTemplateSpecializationType>(LhsT) that will assert for you)
| template<False W> using BFoo = AFoo<W>; | ||
| template<False W> | ||
| using BFoo = AFoo<W>; // expected-note {{candidate template ignored: constraints not satisfied [with V = int]}} \ | ||
| // expected-note {{because '__is_deducible(BFoo, Foo<int *>)' evaluated to false}} \ |
There was a problem hiding this comment.
I think we want to add code in this PR to improve the diagnostic
There was a problem hiding this comment.
Do you think this is a blocker for landing this PR? The current state is acceptable to me, although not ideal. I plan to send out a follow-up patch to address it.
There was a problem hiding this comment.
I certainly think it's not a good user-facing diagnostic, and i don't think the fix would be difficult
but doing it separately seems reasonable. WDYT @AaronBallman?
There was a problem hiding this comment.
I'd prefer to do it in a follow up (this patch is quite large already). Added a FIXME.
| @@ -27,7 +27,8 @@ enum TypeTrait { | |||
| , | |||
| #define TYPE_TRAIT_2(Spelling, Name, Key) BTT_##Name, | |||
| #include "clang/Basic/TokenKinds.def" | |||
| BTT_Last = UTT_Last // BTT_Last == last BTT_XX in the enum. | |||
| BTT_Last = UTT_Last | |||
There was a problem hiding this comment.
oops, this is not intended. This was left when I reverted some changes.
| /// Complete template argument deduction for DeduceTemplateArgumentsFromType. | ||
| /// FIXME: this is mostly duplicated with the above two versions. Deduplicate | ||
| /// the three implementations. |
There was a problem hiding this comment.
It is in my TODO list, but its priority is currently low, so I don't anticipate addressing it in the near future. This seems like a good candidate for a "GoodFirst" issue, so I'm thinking we can file an issue in case someone wants to contribute.
| TemplateDeductionResult | ||
| Sema::DeduceTemplateArgumentsFromType(TemplateDecl *TD, QualType FromType, |
There was a problem hiding this comment.
This function is public (the documentation and comments are in Sema.h header).
| template<False W> using BFoo = AFoo<W>; | ||
| template<False W> | ||
| using BFoo = AFoo<W>; // expected-note {{candidate template ignored: constraints not satisfied [with V = int]}} \ | ||
| // expected-note {{because '__is_deducible(BFoo, Foo<int *>)' evaluated to false}} \ |
There was a problem hiding this comment.
Do you think this is a blocker for landing this PR? The current state is acceptable to me, although not ideal. I plan to send out a follow-up patch to address it.
clang/lib/Sema/SemaTemplate.cpp
Outdated
| FunctionTemplateDecl *FTD, | ||
| llvm::ArrayRef<TemplateArgument> TransformedArgs, | ||
| QualType ReturnType) { | ||
| auto &Context = SemaRef.Context; |
There was a problem hiding this comment.
I'm not seeing a change here?
…templates. Fixes llvm#85192 Fixes llvm#84492 - rebase to main - add release note for __is_deducible - implement diagnostics for bad argument types for __is_deducible Don't expose __is_deducible trait. Refine the implementation of hiding __is_deducible type trait. Apply approach 3.
hokein
force-pushed
the
ctad-is-deducible
branch
3 times, most recently
from
2d8720a
to
c4067b0
Compare
…templates (llvm#89358) Fixes llvm#85192 Fixes llvm#84492 This patch implements the "IsDeducible" constraint where the template arguments of the alias template can be deduced from the returned type of the synthesized deduction guide, per C++ [over.match.class.deduct]p4. In the implementation, we perform the deduction directly, which is more efficient than the way specified in the standard. Also update relevant CTAD tests which were incorrectly compiled due to the missing constraint.
The dictionary entry `=""` is invalid.
Fixes #85192
Fixes #84492
This patch implements the "IsDeducible" constraint where the template arguments of the alias template can be deduced from the returned type of the synthesized deduction guide, per C++ [over.match.class.deduct]p4. In the implementation, we perform the deduction directly, which is more efficient than the way specified in the standard.
Also update relevant CTAD tests which were incorrectly compiled due to the missing constraint.