-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[Clang] Defer the instantiation of explicit-specifier until constraint checking completes #70548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang] Defer the instantiation of explicit-specifier until constraint checking completes #70548
Conversation
@llvm/pr-subscribers-clang Author: 刘雨培 (LYP951018) ChangesModifications:
This PR doesn’t defer the instantiation of the explicit specifier for deduction guides, because I’m not familiar with deduction guides yet. I’ll dig into it after this PR. According to my local test, GCC 13 tuple works with this PR. Fixes #59827. Full diff: https://github.com/llvm/llvm-project/pull/70548.diff 5 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2595737e8b3b143..ad41a831477f66c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -670,6 +670,10 @@ Bug Fixes to C++ Support
default initializing a base class in a constant expression context. Fixes:
(`#69890 <https://github.com/llvm/llvm-project/issues/69890>`_)
+- Clang now defers the instantiation of explicit specifier until constraint checking
+ completes (except deduction guides). Fixes:
+ (`#59827 <https://github.com/llvm/llvm-project/issues/59827>`_)
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
- Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1e9752345ffd173..93fd6507db3d78a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10430,6 +10430,9 @@ class Sema final {
const CXXConstructorDecl *Tmpl,
const MultiLevelTemplateArgumentList &TemplateArgs);
+ ExplicitSpecifier instantiateExplicitSpecifier(
+ const MultiLevelTemplateArgumentList &TemplateArgs, ExplicitSpecifier ES);
+
NamedDecl *FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
const MultiLevelTemplateArgumentList &TemplateArgs,
bool FindingInstantiatedContext = false);
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 0b3f0247ea3bee3..e75e46aa5228812 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3553,6 +3553,53 @@ static unsigned getPackIndexForParam(Sema &S,
llvm_unreachable("parameter index would not be produced from template");
}
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+tryInstantiateExplicitSpecifier(Sema &S, FunctionDecl *Specialization,
+ const MultiLevelTemplateArgumentList &SubstArgs,
+ TemplateDeductionInfo &Info,
+ FunctionTemplateDecl *FunctionTemplate,
+ ArrayRef<TemplateArgument> DeducedArgs) {
+
+ const auto TryInstantiateExplicitSpecifierForSingleDecl =
+ [&](auto *ExplicitDecl) {
+ ExplicitSpecifier ExplicitSpecifier =
+ ExplicitDecl->getExplicitSpecifier();
+ Expr *const Expr = ExplicitSpecifier.getExpr();
+ if (!Expr) {
+ return Sema::TDK_Success;
+ }
+ if (!Expr->isValueDependent()) {
+ return Sema::TDK_Success;
+ }
+ Sema::InstantiatingTemplate Inst(
+ S, Info.getLocation(), FunctionTemplate, DeducedArgs,
+ Sema::CodeSynthesisContext::DeducedTemplateArgumentSubstitution,
+ Info);
+ const auto Instantiated =
+ S.instantiateExplicitSpecifier(SubstArgs, ExplicitSpecifier);
+ if (Instantiated.isInvalid()) {
+ ExplicitDecl->setInvalidDecl(true);
+ return clang::Sema::TDK_SubstitutionFailure;
+ }
+ ExplicitDecl->setExplicitSpecifier(Instantiated);
+ return clang::Sema::TDK_Success;
+ };
+ Sema::TemplateDeductionResult DeductionResult = clang::Sema::TDK_Success;
+ if (CXXConstructorDecl *ConstructorDecl =
+ dyn_cast_or_null<CXXConstructorDecl>(Specialization)) {
+ DeductionResult =
+ TryInstantiateExplicitSpecifierForSingleDecl(ConstructorDecl);
+ } else if (CXXConversionDecl *ConversionDecl =
+ dyn_cast_or_null<CXXConversionDecl>(Specialization)) {
+ DeductionResult =
+ TryInstantiateExplicitSpecifierForSingleDecl(ConversionDecl);
+ }
+ return DeductionResult;
+}
+
/// Finish template argument deduction for a function template,
/// checking the deduced template arguments for completeness and forming
/// the function template specialization.
@@ -3682,6 +3729,15 @@ Sema::TemplateDeductionResult Sema::FinishTemplateArgumentDeduction(
}
}
+ // We skipped the instantiation of the explicit-specifier during subst the
+ // decl before. Now, we try to instantiate it back if the Specialization is a
+ // constructor or a conversion.
+ if (TDK_Success !=
+ tryInstantiateExplicitSpecifier(*this, Specialization, SubstArgs, Info,
+ FunctionTemplate, DeducedArgs)) {
+ return TDK_SubstitutionFailure;
+ }
+
if (OriginalCallArgs) {
// C++ [temp.deduct.call]p4:
// In general, the deduction process attempts to find template argument
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 78a7892a35a320b..163dbef03e80a4d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -563,18 +563,16 @@ static void instantiateDependentAMDGPUFlatWorkGroupSizeAttr(
S.addAMDGPUFlatWorkGroupSizeAttr(New, Attr, MinExpr, MaxExpr);
}
-static ExplicitSpecifier
-instantiateExplicitSpecifier(Sema &S,
- const MultiLevelTemplateArgumentList &TemplateArgs,
- ExplicitSpecifier ES, FunctionDecl *New) {
+ExplicitSpecifier Sema::instantiateExplicitSpecifier(
+ const MultiLevelTemplateArgumentList &TemplateArgs, ExplicitSpecifier ES) {
if (!ES.getExpr())
return ES;
Expr *OldCond = ES.getExpr();
Expr *Cond = nullptr;
{
EnterExpressionEvaluationContext Unevaluated(
- S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
- ExprResult SubstResult = S.SubstExpr(OldCond, TemplateArgs);
+ *this, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+ ExprResult SubstResult = SubstExpr(OldCond, TemplateArgs);
if (SubstResult.isInvalid()) {
return ExplicitSpecifier::Invalid();
}
@@ -582,7 +580,7 @@ instantiateExplicitSpecifier(Sema &S,
}
ExplicitSpecifier Result(Cond, ES.getKind());
if (!Cond->isTypeDependent())
- S.tryResolveExplicitSpecifier(Result);
+ tryResolveExplicitSpecifier(Result);
return Result;
}
@@ -2073,8 +2071,8 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
ExplicitSpecifier InstantiatedExplicitSpecifier;
if (auto *DGuide = dyn_cast<CXXDeductionGuideDecl>(D)) {
- InstantiatedExplicitSpecifier = instantiateExplicitSpecifier(
- SemaRef, TemplateArgs, DGuide->getExplicitSpecifier(), DGuide);
+ InstantiatedExplicitSpecifier = SemaRef.instantiateExplicitSpecifier(
+ TemplateArgs, DGuide->getExplicitSpecifier());
if (InstantiatedExplicitSpecifier.isInvalid())
return nullptr;
}
@@ -2453,11 +2451,25 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
}
}
- ExplicitSpecifier InstantiatedExplicitSpecifier =
- instantiateExplicitSpecifier(SemaRef, TemplateArgs,
- ExplicitSpecifier::getFromDecl(D), D);
- if (InstantiatedExplicitSpecifier.isInvalid())
- return nullptr;
+ auto InstantiatedExplicitSpecifier = ExplicitSpecifier::getFromDecl(D);
+ // deduction guides need this
+ const bool CouldInstantiate =
+ InstantiatedExplicitSpecifier.getExpr() == nullptr ||
+ !InstantiatedExplicitSpecifier.getExpr()->isValueDependent();
+
+ // Delay the instantiation of the explicit-specifier until after the
+ // constraints are checked during template argument deduction.
+ if (CouldInstantiate ||
+ SemaRef.CodeSynthesisContexts.back().Kind !=
+ Sema::CodeSynthesisContext::DeducedTemplateArgumentSubstitution) {
+ InstantiatedExplicitSpecifier = SemaRef.instantiateExplicitSpecifier(
+ TemplateArgs, InstantiatedExplicitSpecifier);
+
+ if (InstantiatedExplicitSpecifier.isInvalid())
+ return nullptr;
+ } else {
+ InstantiatedExplicitSpecifier.setKind(ExplicitSpecKind::Unresolved);
+ }
// Implicit destructors/constructors created for local classes in
// DeclareImplicit* (see SemaDeclCXX.cpp) might not have an associated TSI.
diff --git a/clang/test/SemaCXX/cxx2a-explicit-bool-deferred.cpp b/clang/test/SemaCXX/cxx2a-explicit-bool-deferred.cpp
new file mode 100644
index 000000000000000..4d667008f2e2763
--- /dev/null
+++ b/clang/test/SemaCXX/cxx2a-explicit-bool-deferred.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+
+template <typename T1, typename T2> struct is_same {
+ static constexpr bool value = false;
+};
+
+template <typename T> struct is_same<T, T> {
+ static constexpr bool value = true;
+};
+
+template <class T, class U>
+concept SameHelper = is_same<T, U>::value;
+template <class T, class U>
+concept same_as = SameHelper<T, U> && SameHelper<U, T>;
+
+namespace deferred_instantiation {
+template <class X> constexpr X do_not_instantiate() { return nullptr; }
+
+struct T {
+ template <same_as<float> X> explicit(do_not_instantiate<X>()) T(X) {}
+
+ T(int) {}
+};
+
+T t(5);
+// expected-error@17{{cannot initialize}}
+// expected-note@20{{in instantiation of function template specialization}}
+// expected-note@30{{while substituting deduced template arguments}}
+// expected-note@30{{in instantiation of function template specialization}}
+T t2(5.0f);
+} // namespace deferred_instantiation
|
4f8111b
to
a5929dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this! Some nit comments, hope you don't mind.
(Also invited some clang folks to have a detailed look at this. :=)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine to me, please give other reviewers another day to take a look before merging though.
9912122
to
522998e
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
961ab6c
to
e6ed716
Compare
bba3006
to
0ef00fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a better run-through again, so these are all thats left for me. Thanks for the patience!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want to make sure we have enough testing in place for this change but overall I think I am happy.
Sema::SFINAETrap Trap(S); | ||
const ExplicitSpecifier InstantiatedES = | ||
S.instantiateExplicitSpecifier(SubstArgs, ES); | ||
if (InstantiatedES.isInvalid() || Trap.hasErrorOccurred()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a test that checks this case at all? Same for the other failures, if we know how to test them we should write a test to verify them.
35d0d68
to
aaa687e
Compare
…t checking completes (llvm#70548) Modifications: - Skip the instantiation of the explicit-specifier during Decl substitution if we are deducing template arguments and the explicit-specifier is value dependent. - Instantiate the explicit-specifier after the constraint checking completes. - Make `instantiateExplicitSpecifier` a member function in order to instantiate the explicit-specifier in different stages. This PR doesn’t defer the instantiation of the explicit specifier for deduction guides, because I’m not familiar with deduction guides yet. I’ll dig into it after this PR. According to my local test, GCC 13 tuple works with this PR. Fixes llvm#59827. --------- Co-authored-by: Erich Keane <[email protected]>
Modifications:
Skip the instantiation of the explicit-specifier during Decl substitution if we are deducing template arguments and the explicit-specifier is value dependent.
Instantiate the explicit-specifier after the constraint checking completes.
Make
instantiateExplicitSpecifier
a member function in order to instantiate the explicit-specifier in different stages.This PR doesn’t defer the instantiation of the explicit specifier for deduction guides, because I’m not familiar with deduction guides yet. I’ll dig into it after this PR.
According to my local test, GCC 13 tuple works with this PR.
Fixes #59827.