-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Sema] Default arguments for template parameters affect ContainsUnexpandedPacks #99880
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
Conversation
@llvm/pr-subscribers-clang Author: Ilya Biryukov (ilya-biryukov) ChangesThis addresses the FIXME in the code. There are tests for the new behavior in a follow up fix for #99877, which also addresses other bugs that prevent exposing the wrong results of Full diff: https://github.com/llvm/llvm-project/pull/99880.diff 1 Files Affected:
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 722c7fcf0b0df..f95be88e6c087 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -61,27 +61,43 @@ TemplateParameterList::TemplateParameterList(const ASTContext& C,
bool IsPack = P->isTemplateParameterPack();
if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P)) {
- if (!IsPack && NTTP->getType()->containsUnexpandedParameterPack())
- ContainsUnexpandedParameterPack = true;
+ if (!IsPack) {
+ if (NTTP->getType()->containsUnexpandedParameterPack())
+ ContainsUnexpandedParameterPack = true;
+ else if (NTTP->hasDefaultArgument() &&
+ NTTP->getDefaultArgument()
+ .getArgument()
+ .containsUnexpandedParameterPack())
+ ContainsUnexpandedParameterPack = true;
+ }
if (NTTP->hasPlaceholderTypeConstraint())
HasConstrainedParameters = true;
} else if (const auto *TTP = dyn_cast<TemplateTemplateParmDecl>(P)) {
- if (!IsPack &&
- TTP->getTemplateParameters()->containsUnexpandedParameterPack())
- ContainsUnexpandedParameterPack = true;
- } else if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(P)) {
- if (const TypeConstraint *TC = TTP->getTypeConstraint()) {
- if (TC->getImmediatelyDeclaredConstraint()
- ->containsUnexpandedParameterPack())
+ if (!IsPack) {
+ if (TTP->getTemplateParameters()->containsUnexpandedParameterPack())
ContainsUnexpandedParameterPack = true;
+ else if (TTP->hasDefaultArgument() &&
+ TTP->getDefaultArgument()
+ .getArgument()
+ .containsUnexpandedParameterPack())
+ ContainsUnexpandedParameterPack = true;
+ }
+ } else if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(P)) {
+ if (!IsPack && TTP->hasDefaultArgument() &&
+ TTP->getDefaultArgument()
+ .getArgument()
+ .containsUnexpandedParameterPack()) {
+ ContainsUnexpandedParameterPack = true;
+ } else if (const TypeConstraint *TC = TTP->getTypeConstraint();
+ TC && TC->getImmediatelyDeclaredConstraint()
+ ->containsUnexpandedParameterPack()) {
+ ContainsUnexpandedParameterPack = true;
}
if (TTP->hasTypeConstraint())
HasConstrainedParameters = true;
} else {
llvm_unreachable("unexpected template parameter type");
}
- // FIXME: If a default argument contains an unexpanded parameter pack, the
- // template parameter list does too.
}
if (HasRequiresClause) {
|
clang/lib/AST/DeclTemplate.cpp
Outdated
NTTP->getDefaultArgument() | ||
.getArgument() | ||
.containsUnexpandedParameterPack()) |
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.
could we have a function - or at least a lambda - that avoid the repetition of that 3 times?
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.
Done.
…andedPacks This addresses the FIXME in the code. There are tests for the new behavior in a follow up fix for llvm#99877, which also addresses other bugs that prevent exposing the wrong results of `ContainsUnexpandedPacks` in the outputs of the compiler without crashes.
…nsUnexpandedPacks Add a helper function to avoid code duplication.
6a7422e
to
34a18e0
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
…nsUnexpandedPacks reformat the code
…nsUnexpandedPacks Simplify the code and reduce nesting
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.
Modulo nitpicking I failed to do earlier this looks great.
Thanks!
…nsUnexpandedPacks - Rename function - Use static instead of anonymous namespace
a3c6b24
to
d64b95c
Compare
…andedPacks (#99880) Summary: This addresses the FIXME in the code. There will be tests for the new behavior in an upcoming #86265, which also addresses other bugs that prevent exposing the wrong results of `ContainsUnexpandedPacks` in the outputs of the compiler without crashes. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251126
This addresses the FIXME in the code. There will be tests for the new behavior in an upcoming #86265, which also addresses other bugs that prevent exposing the wrong results of
ContainsUnexpandedPacks
in the outputs of the compiler without crashes.