[clang] Use canonical type for substitution which might be incomplete #109065
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When checking deduction consistency, a substitution can be incomplete such that only sugar parts refer to non-deduced template parameters. This would not otherwise lead to an inconsistent deduction, so this patch makes it so we canonicalize the types before substitution in order to avoid that possibility, for now. When we are able to produce substitution failure diagnostics for partial ordering, we might want to improve the TemplateInstantiator so that it does not fail in that case. This fixes a regression on top of #100692, which was reported on the PR. This was never released, so there are no release notes.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesWhen checking deduction consistency, a substitution can be incomplete such that only sugar parts refer to non-deduced template parameters. This would not otherwise lead to an inconsistent deduction, so this patch makes it so we canonicalize the types before substitution in order to avoid that possibility, for now. When we are able to produce substitution failure diagnostics for partial ordering, we might want to improve the TemplateInstantiator so that it does not fail in that case. This fixes a regression on top of #100692, which was reported on the PR. This was never released, so there are no release notes. Full diff: https://github.com/llvm/llvm-project/pull/109065.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index b50648d5752ce5..7d83b86a007337 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5505,8 +5505,11 @@ static TemplateDeductionResult CheckDeductionConsistency(
Sema::ArgumentPackSubstitutionIndexRAII PackIndex(
S, ArgIdx != -1 ? ::getPackIndexForParam(S, FTD, MLTAL, ArgIdx) : -1);
bool IsIncompleteSubstitution = false;
- QualType InstP = S.SubstType(P, MLTAL, FTD->getLocation(), FTD->getDeclName(),
- &IsIncompleteSubstitution);
+ // FIXME: A substitution can be incomplete on a non-structural part of the
+ // type. Use the canonical type for now, until the TemplateInstantiator can
+ // deal with that.
+ QualType InstP = S.SubstType(P.getCanonicalType(), MLTAL, FTD->getLocation(),
+ FTD->getDeclName(), &IsIncompleteSubstitution);
if (InstP.isNull() && !IsIncompleteSubstitution)
return TemplateDeductionResult::SubstitutionFailure;
if (!CheckConsistency)
diff --git a/clang/test/SemaTemplate/GH18291.cpp b/clang/test/SemaTemplate/GH18291.cpp
index 820564ffa6f1a0..2e9754b6561740 100644
--- a/clang/test/SemaTemplate/GH18291.cpp
+++ b/clang/test/SemaTemplate/GH18291.cpp
@@ -112,3 +112,12 @@ namespace static_vs_nonstatic {
}
} // namespace explicit_obj_param
} // namespace static_vs_nonstatic
+
+namespace incomplete_on_sugar {
+ template <unsigned P, class T> void f(T[P]) = delete;
+ template <unsigned P> void f(int[][P]);
+ void test() {
+ int array[1][8];
+ f<8>(array);
+ }
+} // namespace incomplete_on_sugar
|
alexfh
approved these changes
Sep 18, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/4770 Here is the relevant piece of the build log for the reference |
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
…llvm#109065) When checking deduction consistency, a substitution can be incomplete such that only sugar parts refer to non-deduced template parameters. This would not otherwise lead to an inconsistent deduction, so this patch makes it so we canonicalize the types before substitution in order to avoid that possibility, for now. When we are able to produce substitution failure diagnostics for partial ordering, we might want to improve the TemplateInstantiator so that it does not fail in that case. This fixes a regression on top of llvm#100692, which was reported on the PR. This was never released, so there are no release notes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When checking deduction consistency, a substitution can be incomplete such that only sugar parts refer to non-deduced template parameters.
This would not otherwise lead to an inconsistent deduction, so this patch makes it so we canonicalize the types before substitution in order to avoid that possibility, for now.
When we are able to produce substitution failure diagnostics for partial ordering, we might want to improve the TemplateInstantiator so that it does not fail in that case.
This fixes a regression on top of #100692, which was reported on the PR. This was never released, so there are no release notes.