[Clang] Ensure initialized NTTP expressions when building CTAD for type aliases #161035
Conversation
This file contains hidden or 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
…pe aliases We missed calling CheckTemplateArgument when building CTAD deduction guides. That ensures some InitExprs are correctly initialized, as in the test that crashed due to incorrect NTTP initialization. We don't use CheckTemplateArguments because, in CTAD synthesis, template parameter packs don't always appear at the end of the parameter list, unlike user-written ones mandated by the standard. This makes it difficult for CheckTemplateArguments to determine how many arguments a pack should match, leading to unnecessary complexity. On the other hand, since we substitute non-deduced template parameters with deduced ones, we need to fold the packs midway through substitution, where CheckTemplateArgument is more convenient. As a drive-by this also removes some dead code in SemaInit.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesWe missed calling CheckTemplateArgument when building CTAD deduction guides. That ensures some InitExprs are correctly initialized, as in the test that crashed due to incorrect NTTP initialization. I don't use CheckTemplateArguments because, in CTAD synthesis, template parameter packs don't always appear at the end of the parameter list, unlike user-written ones mandated by the standard. This makes it difficult for CheckTemplateArguments to determine how many arguments a pack in middle should match, leading to unnecessary complexity. On the other hand, since we substitute non-deduced template parameters with deduced ones, we need to fold the packs midway through substitution, where CheckTemplateArgument is more convenient. As a drive-by this also removes some dead code in SemaInit. Fixes #131408 Full diff: https://github.com/llvm/llvm-project/pull/161035.diff 4 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 98c889c08b329..888b202c538de 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -361,7 +361,7 @@ Bug Fixes in This Version
first parameter. (#GH113323).
- Fixed a crash with incompatible pointer to integer conversions in designated
initializers involving string literals. (#GH154046)
-- Fix crash on CTAD for alias template. (#GH131342)
+- Fix crash on CTAD for alias template. (#GH131342), (#GH131408)
- Clang now emits a frontend error when a function marked with the `flatten` attribute
calls another function that requires target features not enabled in the caller. This
prevents a fatal error in the backend.
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index c97129336736b..0d0d2c00eb5d4 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -8219,8 +8219,8 @@ ExprResult InitializationSequence::Perform(Sema &S,
// InitializeTemporary entity for our target type.
QualType Ty = Step->Type;
bool IsTemporary = !S.Context.hasSameType(Entity.getType(), Ty);
- InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(Ty);
- InitializedEntity InitEntity = IsTemporary ? TempEntity : Entity;
+ InitializedEntity InitEntity =
+ IsTemporary ? InitializedEntity::InitializeTemporary(Ty) : Entity;
InitListChecker PerformInitList(S, InitEntity,
InitList, Ty, /*VerifyOnly=*/false,
/*TreatUnavailableAsInvalid=*/false);
@@ -8242,7 +8242,6 @@ ExprResult InitializationSequence::Perform(Sema &S,
InitListExpr *StructuredInitList =
PerformInitList.getFullyStructuredList();
- CurInit.get();
CurInit = shouldBindAsTemporary(InitEntity)
? S.MaybeBindToTemporary(StructuredInitList)
: StructuredInitList;
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index 3d54d1eb4373a..451ec3754ca70 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -1171,17 +1171,38 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
const auto &D = DeduceResults[Index];
+ auto *TP = F->getTemplateParameters()->getParam(Index);
if (IsNonDeducedArgument(D)) {
// 2): Non-deduced template parameters would be substituted later.
continue;
}
TemplateArgumentLoc Input =
SemaRef.getTrivialTemplateArgumentLoc(D, QualType(), SourceLocation{});
- TemplateArgumentLoc Output;
- if (!SemaRef.SubstTemplateArgument(Input, Args, Output)) {
+ TemplateArgumentListInfo Output;
+ if (!SemaRef.SubstTemplateArguments(Input, Args, Output)) {
assert(TemplateArgsForBuildingFPrime[Index].isNull() &&
"InstantiatedArgs must be null before setting");
- TemplateArgsForBuildingFPrime[Index] = Output.getArgument();
+ Sema::CheckTemplateArgumentInfo CTAI;
+ if (Input.getArgument().getKind() == TemplateArgument::Pack) {
+ for (auto TA : Output.arguments()) {
+ if (SemaRef.CheckTemplateArgument(
+ TP, TA, F, F->getLocation(), F->getLocation(),
+ /*ArgumentPackIndex=*/-1, CTAI,
+ Sema::CheckTemplateArgumentKind::CTAK_Specified))
+ return nullptr;
+ }
+ TemplateArgsForBuildingFPrime[Index] =
+ TemplateArgument::CreatePackCopy(Context, CTAI.SugaredConverted);
+ } else {
+ assert(Output.arguments().size() == 1);
+ TemplateArgumentLoc Transformed = Output.arguments()[0];
+ if (SemaRef.CheckTemplateArgument(
+ TP, Transformed, F, F->getLocation(), F->getLocation(),
+ /*ArgumentPackIndex=*/-1, CTAI,
+ Sema::CheckTemplateArgumentKind::CTAK_Specified))
+ return nullptr;
+ TemplateArgsForBuildingFPrime[Index] = CTAI.SugaredConverted[0];
+ }
}
}
@@ -1202,8 +1223,21 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
assert(TemplateArgsForBuildingFPrime[FTemplateParamIdx].isNull() &&
"The argument must be null before setting");
+ TemplateArgument Transformed = Context.getInjectedTemplateArg(NewParam);
+ if (NewParam->isTemplateParameterPack())
+ Transformed = *Transformed.pack_begin();
+ TemplateArgumentLoc TALoc = SemaRef.getTrivialTemplateArgumentLoc(
+ Transformed, QualType(), NewParam->getBeginLoc());
+ Sema::CheckTemplateArgumentInfo CTAI;
+ if (SemaRef.CheckTemplateArgument(
+ TP, TALoc, F, F->getLocation(), F->getLocation(),
+ /*ArgumentPackIndex=*/-1, CTAI,
+ Sema::CheckTemplateArgumentKind::CTAK_Specified))
+ return nullptr;
TemplateArgsForBuildingFPrime[FTemplateParamIdx] =
- Context.getInjectedTemplateArg(NewParam);
+ NewParam->isTemplateParameterPack()
+ ? TemplateArgument::CreatePackCopy(Context, CTAI.SugaredConverted)
+ : CTAI.SugaredConverted[0];
}
auto *TemplateArgListForBuildingFPrime =
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index 2f1817d0ca7eb..d7ce966f46060 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -586,3 +586,21 @@ Baz a{};
static_assert(__is_same(decltype(a), A<A<int>>));
} // namespace GH133132
+
+namespace GH131408 {
+
+struct Node {};
+
+template <class T, Node>
+struct A {
+ A(T) {}
+};
+
+template <class T>
+using AA = A<T, {}>;
+
+AA a{0};
+
+static_assert(__is_same(decltype(a), A<int, Node{}>));
+
+}
|
mizvekov
reviewed
Sep 29, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/128/builds/7471 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/23492 Here is the relevant piece of the build log for the reference |
MixedMatched
pushed a commit
to MixedMatched/llvm-project
that referenced
this pull request
Oct 3, 2025
…pe aliases (llvm#161035) We missed calling CheckTemplateArgument when building CTAD deduction guides. That ensures some InitExprs are correctly initialized, as in the test that crashed due to incorrect NTTP initialization. I don't use CheckTemplateArguments because, in CTAD synthesis, template parameter packs don't always appear at the end of the parameter list, unlike user-written ones mandated by the standard. This makes it difficult for CheckTemplateArguments to determine how many arguments a pack in middle should match, leading to unnecessary complexity. On the other hand, since we substitute non-deduced template parameters with deduced ones, we need to fold the packs midway through substitution, where CheckTemplateArgument is more convenient. As a drive-by this also removes some dead code in SemaInit. Fixes llvm#131408
shafik
reviewed
Oct 3, 2025
| } else { | ||
| assert(Output.arguments().size() == 1); | ||
| TemplateArgumentLoc Transformed = Output.arguments()[0]; | ||
| if (SemaRef.CheckTemplateArgument( |
There was a problem hiding this comment.
I am looking at the for loop above and it sure seems like you could also use the for loop here and just these both above the if and remove the redundant code.
There was a problem hiding this comment.
Thanks, I filed #161948
zyn0217
added a commit
that referenced
this pull request
Oct 4, 2025
…61948) This addresses the post-commit review #161035 (comment) from Shafik
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 4, 2025
…peAlias (#161948) This addresses the post-commit review llvm/llvm-project#161035 (comment) from Shafik
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
We missed calling CheckTemplateArgument when building CTAD deduction guides. That ensures some InitExprs are correctly initialized, as in the test that crashed due to incorrect NTTP initialization.
I don't use CheckTemplateArguments because, in CTAD synthesis, template parameter packs don't always appear at the end of the parameter list, unlike user-written ones mandated by the standard. This makes it difficult for CheckTemplateArguments to determine how many arguments a pack in middle should match, leading to unnecessary complexity.
On the other hand, since we substitute non-deduced template parameters with deduced ones, we need to fold the packs midway through substitution, where CheckTemplateArgument is more convenient.
As a drive-by this also removes some dead code in SemaInit.
Fixes #131408