[Clang] Do not try to transform invalid bindings #125658
Merged
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) ChangesIn the presence of an invalid structured binding decomposition, some binding packs may be invalid and trying to transform them would produce a recovery expression that does not contains a pack, leading to assertions in places where we would expect a pack at that stage. Fixes #125165 Full diff: https://github.com/llvm/llvm-project/pull/125658.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index dc3bfa97eff399..fb3df8fbd39075 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2481,7 +2481,7 @@ TemplateInstantiator::TransformDeclRefExpr(DeclRefExpr *E) {
if (BindingDecl *BD = dyn_cast<BindingDecl>(D); BD && BD->isParameterPack()) {
BD = cast_or_null<BindingDecl>(TransformDecl(BD->getLocation(), BD));
- if (!BD)
+ if (!BD || BD->isInvalidDecl())
return ExprError();
if (auto *RP =
dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding()))
diff --git a/clang/test/SemaCXX/cxx2c-binding-pack.cpp b/clang/test/SemaCXX/cxx2c-binding-pack.cpp
index 5ca249f52b3d8e..3340243f7be583 100644
--- a/clang/test/SemaCXX/cxx2c-binding-pack.cpp
+++ b/clang/test/SemaCXX/cxx2c-binding-pack.cpp
@@ -188,3 +188,19 @@ void other_main() {
static_assert(f<int>() == 2);
}
} // namespace
+
+
+namespace GH125165 {
+
+template <typename = void>
+auto f(auto t) {
+ const auto& [...pack] = t;
+ // expected-error@-1 {{cannot decompose non-class, non-array type 'char const'}}
+ (pack, ...);
+};
+
+void g() {
+ f('x'); // expected-note {{in instantiation}}
+}
+
+}
|
2 tasks
Sirraide
approved these changes
Feb 4, 2025
|
Oh actually, one thing to note is, how is this going to interact with #125394 since that just deletes the entire if statement that this change is part of? |
By that I mean the one starting on line 2482 |
Yeah, maybe it was not smart of me to work on that! The other way to fix that is to move some of the checks in Somewhere there https://github.com/llvm/llvm-project/pull/125394/files#diff-dae8060cd0876af2d1e1376f5da83d22a01fd1b49730cee2deaee9727e64e0e4R2415 ? |
Yeah, I’d say so too |
erichkeane
approved these changes
Feb 4, 2025
shafik
reviewed
Feb 6, 2025
| }; | ||
|
|
||
| void g() { | ||
| f('x'); // expected-note {{in instantiation}} |
There was a problem hiding this comment.
Interesting, should this work: https://godbolt.org/z/dKcaa3jY4
We should also try other arguments types beside char like int, float etc
There was a problem hiding this comment.
That happens to work because the pack can be empty. In clang-19 it seems to still allow decomposition. https://godbolt.org/z/zMrqTYE8c
(I am not saying that it should though.... maybe because it is empty?)
There was a problem hiding this comment.
In the presence of an invalid structured binding decomposition, some binding packs may be invalid and trying to transform them would produce a recovery expression that does not contains a pack, leading to assertions in places where we would expect a pack at that stage. Fixes llvm#125165
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/13280 Here is the relevant piece of the build log for the reference |
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.
In the presence of an invalid structured binding decomposition, some binding packs may be invalid and trying to transform them would produce a recovery expression that does not contains a pack, leading to assertions in places where we would expect a pack at that stage.
Fixes #125165