[Clang][Sema] Fix wrong initialization kind when handling initializing structured bindings from an array with direct-list-initialization #124793
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
Member
|
@llvm/pr-subscribers-clang Author: Yanzuo Liu (zwuis) ChangesIn my PR #102581, elements of structured bindings is copy-initialized. They should be direct-initialized because the form of the initializer of the whole structured bindings is direct-list-initialization. [[dcl.struct.bind]/1](https://eel.is/c++draft/dcl.struct.bind#1.sentence-8): int arr[2]{};
// elements of `[a, b]` should be direct-initialized
auto [a, b]{arr};Full diff: https://github.com/llvm/llvm-project/pull/124793.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index b95cbbf4222056..5552fce55f1310 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4861,8 +4861,9 @@ static void TryListInitialization(Sema &S,
S.Context.hasSameUnqualifiedType(SubInit[0]->getType(), DestType) &&
"Deduced to other type?");
TryArrayCopy(S,
- InitializationKind::CreateCopy(Kind.getLocation(),
- InitList->getLBraceLoc()),
+ InitializationKind::CreateDirect(Kind.getLocation(),
+ InitList->getLBraceLoc(),
+ InitList->getRBraceLoc()),
Entity, SubInit[0], DestType, Sequence,
TreatUnavailableAsInvalid);
if (Sequence)
diff --git a/clang/test/SemaCXX/cxx1z-decomposition.cpp b/clang/test/SemaCXX/cxx1z-decomposition.cpp
index a8914fe4e9cd82..b3d98e44990f61 100644
--- a/clang/test/SemaCXX/cxx1z-decomposition.cpp
+++ b/clang/test/SemaCXX/cxx1z-decomposition.cpp
@@ -200,38 +200,37 @@ namespace lambdas {
namespace by_value_array_copy {
struct explicit_copy {
- explicit_copy() = default; // expected-note 2{{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
- explicit explicit_copy(const explicit_copy&) = default; // expected-note 2{{explicit constructor is not a candidate}}
+ explicit_copy() = default; // expected-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
+ explicit explicit_copy(const explicit_copy&) = default; // expected-note {{explicit constructor is not a candidate}}
};
constexpr int direct_initialization_for_elements() {
explicit_copy ec_arr[2];
auto [a1, b1](ec_arr);
+ auto [a2, b2]{ec_arr};
int arr[3]{1, 2, 3};
- auto [a2, b2, c2](arr);
+ auto [a3, b3, c3](arr);
+ auto [a4, b4, c4]{arr}; // GH31813
arr[0]--;
- return a2 + b2 + c2 + arr[0];
+ return a3 + b3 + c3 + a4 + b4 + c4 + arr[0];
}
- static_assert(direct_initialization_for_elements() == 6);
+ static_assert(direct_initialization_for_elements() == 12);
constexpr int copy_initialization_for_elements() {
int arr[2]{4, 5};
auto [a1, b1] = arr;
- auto [a2, b2]{arr}; // GH31813
arr[0] = 0;
- return a1 + b1 + a2 + b2 + arr[0];
+ return a1 + b1 + arr[0];
}
- static_assert(copy_initialization_for_elements() == 18);
+ static_assert(copy_initialization_for_elements() == 9);
void copy_initialization_for_elements_with_explicit_copy_ctor() {
explicit_copy ec_arr[2];
auto [a1, b1] = ec_arr; // expected-error {{no matching constructor for initialization of 'explicit_copy[2]'}}
- auto [a2, b2]{ec_arr}; // expected-error {{no matching constructor for initialization of 'explicit_copy[2]'}}
// Test prvalue
using T = explicit_copy[2];
- auto [a3, b3] = T{};
- auto [a4, b4]{T{}};
+ auto [a2, b2] = T{};
}
} // namespace by_value_array_copy
|
Contributor
Author
|
Release note is not needed if this PR is cherry-picked to |
cor3ntin
reviewed
Jan 28, 2025
Comment on lines
+4864
to
+4866
| InitializationKind::CreateDirect(Kind.getLocation(), | ||
| InitList->getLBraceLoc(), | ||
| InitList->getRBraceLoc()), |
Contributor
There was a problem hiding this comment.
and each element is copy-initialized or direct-initialized from the corresponding element of the assignment-expression as specified by the form of the initializer. Otherwise, e is defined as-if by
So I think
auto [_]{a}; // direct-initialization
auto [_](a); // copy-initializerIs my understanding correct that we need that we need to look at Kind.getKind() and call either CreateCopy or CreateDirect?
Contributor
There was a problem hiding this comment.
I think your understanding is correct.
Collaborator
There was a problem hiding this comment.
This looks correct to me as well, can we come up w/ a test case where it makes a difference though.
Contributor
Author
There was a problem hiding this comment.
... Is my understanding correct that we need that we need to look at
Kind.getKind()and call eitherCreateCopyor CreateDirect?
This part of code is in function trylistinitialization. The function handles list-initialization (for structured bindings, it must be direct-list-initialization here) only.
This looks correct to me as well, can we come up w/ a test case where it makes a difference though.
I believe I have added these tests. I will make these tests more clear.
Contributor
|
Can you add a changelog entry? |
cor3ntin
approved these changes
Feb 4, 2025
Contributor
Author
|
Thank you for your review! I don't have commit access. Please help me merge this PR if it's ready. |
Contributor
|
@zwuis Can you resolve the conflicts before we help you merge it? thanks |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/6756 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/4525 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…g structured bindings from an array with direct-list-initialization (llvm#124793) In 377257f, elements of structured bindings are copy-initialized. They should be direct-initialized because the form of the initializer of the whole structured bindings is a direct-list-initialization. > [dcl.struct.bind]/1: > ... and each element is copy-initialized or direct-initialized from the corresponding element of the assignment-expression as specified by the form of the initializer. ... For example, ```cpp int arr[2]{}; // elements of `[a, b]` should be direct-initialized auto [a, b]{arr}; ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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.
In my PR #102581, elements of structured bindings is copy-initialized. They should be direct-initialized because the form of the initializer of the whole structured bindings is direct-list-initialization.
[dcl.struct.bind]/1: