* [PATCH] c++: Fall through for arrays of T vs T cv [PR104996]
@ 2022-03-22 20:39 Ed Catmur
2022-03-22 20:59 ` Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Ed Catmur @ 2022-03-22 20:39 UTC (permalink / raw)
To: gcc-patches
If two arrays do not have the exact same element type including qualification, this could be e.g. f(int (&&)[]) vs. f(int const (&)[]), which can still be distinguished by the lvalue-rvalue tiebreaker.
By tightening this branch (in accordance with the letter of the Standard) we fall through to the next branch, which tests whether they have different element type ignoring qualification and returns 0 in that case; thus we only actually fall through in the T[...] vs. T cv[...] case, eventually considering the lvalue-rvalue tiebreaker at the end of compare_ics.
Add test.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104996
---
gcc/cp/call.cc | 7 ++-----
gcc/testsuite/g++.dg/pr104996.C | 5 +++++
2 files changed, 7 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr104996.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 23d3fc496b822..28589ab3459ea 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -11535,12 +11535,9 @@ compare_ics (conversion *ics1, conversion *ics2)
P0388R4.) */
else if (t1->kind == ck_aggr
&& TREE_CODE (t1->type) == ARRAY_TYPE
- && TREE_CODE (t2->type) == ARRAY_TYPE)
+ && TREE_CODE (t2->type) == ARRAY_TYPE
+ && same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
{
- /* The type of the array elements must be the same. */
- if (!same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
- return 0;
-
tree n1 = nelts_initialized_by_list_init (t1);
tree n2 = nelts_initialized_by_list_init (t2);
if (tree_int_cst_lt (n1, n2))
diff --git a/gcc/testsuite/g++.dg/pr104996.C b/gcc/testsuite/g++.dg/pr104996.C
new file mode 100644
index 0000000000000..2e7558c7b9c77
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr104996.C
@@ -0,0 +1,5 @@
+// { dg-do compile { target c++11 } }
+
+template<unsigned size> char f(int (&&)[size]);
+template<unsigned size> int f(int const (&)[size]);
+static_assert(sizeof(f({1, 2, 3})) == 1, "");
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fall through for arrays of T vs T cv [PR104996]
2022-03-22 20:39 [PATCH] c++: Fall through for arrays of T vs T cv [PR104996] Ed Catmur
@ 2022-03-22 20:59 ` Marek Polacek
2022-03-24 21:06 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-03-22 20:59 UTC (permalink / raw)
To: Ed Catmur; +Cc: gcc-patches
On Tue, Mar 22, 2022 at 08:39:21PM +0000, Ed Catmur wrote:
> If two arrays do not have the exact same element type including qualification, this could be e.g. f(int (&&)[]) vs. f(int const (&)[]), which can still be distinguished by the lvalue-rvalue tiebreaker.
>
> By tightening this branch (in accordance with the letter of the Standard) we fall through to the next branch, which tests whether they have different element type ignoring qualification and returns 0 in that case; thus we only actually fall through in the T[...] vs. T cv[...] case, eventually considering the lvalue-rvalue tiebreaker at the end of compare_ics.
>
> Add test.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104996
Thanks, the patch looks reasonable. Can you describe how it's been tested?
You're going to need a ChangeLog entry, something like
* call.c (compare_ics): When comparing list-initialization sequences,
do not return early.
and an entry for the test, which...
> ---
> gcc/cp/call.cc | 7 ++-----
> gcc/testsuite/g++.dg/pr104996.C | 5 +++++
> 2 files changed, 7 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/pr104996.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 23d3fc496b822..28589ab3459ea 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -11535,12 +11535,9 @@ compare_ics (conversion *ics1, conversion *ics2)
> P0388R4.) */
> else if (t1->kind == ck_aggr
> && TREE_CODE (t1->type) == ARRAY_TYPE
> - && TREE_CODE (t2->type) == ARRAY_TYPE)
> + && TREE_CODE (t2->type) == ARRAY_TYPE
> + && same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
> {
> - /* The type of the array elements must be the same. */
> - if (!same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
> - return 0;
> -
> tree n1 = nelts_initialized_by_list_init (t1);
> tree n2 = nelts_initialized_by_list_init (t2);
> if (tree_int_cst_lt (n1, n2))
> diff --git a/gcc/testsuite/g++.dg/pr104996.C b/gcc/testsuite/g++.dg/pr104996.C
> new file mode 100644
> index 0000000000000..2e7558c7b9c77
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr104996.C
...can you please rename the test to initlist129.C and put it into g++.dg/cpp0x?
> @@ -0,0 +1,5 @@
Also please add
// PR c++/104996
in the first line.
> +// { dg-do compile { target c++11 } }
> +
> +template<unsigned size> char f(int (&&)[size]);
> +template<unsigned size> int f(int const (&)[size]);
> +static_assert(sizeof(f({1, 2, 3})) == 1, "");
>
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fall through for arrays of T vs T cv [PR104996]
2022-03-22 20:59 ` Marek Polacek
@ 2022-03-24 21:06 ` Jason Merrill
2022-04-12 14:32 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-03-24 21:06 UTC (permalink / raw)
To: Marek Polacek, Ed Catmur; +Cc: gcc-patches
On 3/22/22 16:59, Marek Polacek via Gcc-patches wrote:
> On Tue, Mar 22, 2022 at 08:39:21PM +0000, Ed Catmur wrote:
>> If two arrays do not have the exact same element type including qualification, this could be e.g. f(int (&&)[]) vs. f(int const (&)[]), which can still be distinguished by the lvalue-rvalue tiebreaker.
>>
>> By tightening this branch (in accordance with the letter of the Standard) we fall through to the next branch, which tests whether they have different element type ignoring qualification and returns 0 in that case; thus we only actually fall through in the T[...] vs. T cv[...] case, eventually considering the lvalue-rvalue tiebreaker at the end of compare_ics.
>>
>> Add test.
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104996
>
> Thanks, the patch looks reasonable. Can you describe how it's been tested?
>
> You're going to need a ChangeLog entry, something like
>
> * call.c (compare_ics): When comparing list-initialization sequences,
> do not return early.
Indeed. Note that contrib/gcc-git-customization.sh creates a 'git
gcc-commit-mklog' alias that is useful for generating a ChangeLog skeleton.
Also, you don't seem to have a copyright assignment on file, so please
add a DCO sign-off; see https://gcc.gnu.org/contribute.html#legal
> and an entry for the test, which...
>
>> ---
>> gcc/cp/call.cc | 7 ++-----
>> gcc/testsuite/g++.dg/pr104996.C | 5 +++++
>> 2 files changed, 7 insertions(+), 5 deletions(-)
>> create mode 100644 gcc/testsuite/g++.dg/pr104996.C
>>
>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>> index 23d3fc496b822..28589ab3459ea 100644
>> --- a/gcc/cp/call.cc
>> +++ b/gcc/cp/call.cc
>> @@ -11535,12 +11535,9 @@ compare_ics (conversion *ics1, conversion *ics2)
>> P0388R4.) */
>> else if (t1->kind == ck_aggr
>> && TREE_CODE (t1->type) == ARRAY_TYPE
>> - && TREE_CODE (t2->type) == ARRAY_TYPE)
>> + && TREE_CODE (t2->type) == ARRAY_TYPE
>> + && same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
>> {
>> - /* The type of the array elements must be the same. */
>> - if (!same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
>> - return 0;
>> -
>> tree n1 = nelts_initialized_by_list_init (t1);
>> tree n2 = nelts_initialized_by_list_init (t2);
>> if (tree_int_cst_lt (n1, n2))
>> diff --git a/gcc/testsuite/g++.dg/pr104996.C b/gcc/testsuite/g++.dg/pr104996.C
>> new file mode 100644
>> index 0000000000000..2e7558c7b9c77
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr104996.C
>
> ...can you please rename the test to initlist129.C and put it into g++.dg/cpp0x?
>
>> @@ -0,0 +1,5 @@
>
> Also please add
>
> // PR c++/104996
>
> in the first line.
>
>> +// { dg-do compile { target c++11 } }
>> +
>> +template<unsigned size> char f(int (&&)[size]);
>> +template<unsigned size> int f(int const (&)[size]);
>> +static_assert(sizeof(f({1, 2, 3})) == 1, "");
>>
>
> Marek
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fall through for arrays of T vs T cv [PR104996]
2022-03-24 21:06 ` Jason Merrill
@ 2022-04-12 14:32 ` Jason Merrill
2022-04-18 22:09 ` [PATCH v2] " Ed Catmur
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-04-12 14:32 UTC (permalink / raw)
To: Ed Catmur; +Cc: gcc-patches, Marek Polacek
On 3/24/22 17:06, Jason Merrill wrote:
> On 3/22/22 16:59, Marek Polacek via Gcc-patches wrote:
>> On Tue, Mar 22, 2022 at 08:39:21PM +0000, Ed Catmur wrote:
>>> If two arrays do not have the exact same element type including
>>> qualification, this could be e.g. f(int (&&)[]) vs. f(int const
>>> (&)[]), which can still be distinguished by the lvalue-rvalue
>>> tiebreaker.
>>>
>>> By tightening this branch (in accordance with the letter of the
>>> Standard) we fall through to the next branch, which tests whether
>>> they have different element type ignoring qualification and returns 0
>>> in that case; thus we only actually fall through in the T[...] vs. T
>>> cv[...] case, eventually considering the lvalue-rvalue tiebreaker at
>>> the end of compare_ics.
>>>
>>> Add test.
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104996
>>
>> Thanks, the patch looks reasonable. Can you describe how it's been
>> tested?
>>
>> You're going to need a ChangeLog entry, something like
>>
>> * call.c (compare_ics): When comparing list-initialization sequences,
>> do not return early.
>
> Indeed. Note that contrib/gcc-git-customization.sh creates a 'git
> gcc-commit-mklog' alias that is useful for generating a ChangeLog skeleton.
>
> Also, you don't seem to have a copyright assignment on file, so please
> add a DCO sign-off; see https://gcc.gnu.org/contribute.html#legal
Hi Ed, we're getting close to the GCC release and I would like to
include this patch, just need the above adjustments.
>> and an entry for the test, which...
>>
>>> ---
>>> gcc/cp/call.cc | 7 ++-----
>>> gcc/testsuite/g++.dg/pr104996.C | 5 +++++
>>> 2 files changed, 7 insertions(+), 5 deletions(-)
>>> create mode 100644 gcc/testsuite/g++.dg/pr104996.C
>>>
>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>> index 23d3fc496b822..28589ab3459ea 100644
>>> --- a/gcc/cp/call.cc
>>> +++ b/gcc/cp/call.cc
>>> @@ -11535,12 +11535,9 @@ compare_ics (conversion *ics1, conversion
>>> *ics2)
>>> P0388R4.) */
>>> else if (t1->kind == ck_aggr
>>> && TREE_CODE (t1->type) == ARRAY_TYPE
>>> - && TREE_CODE (t2->type) == ARRAY_TYPE)
>>> + && TREE_CODE (t2->type) == ARRAY_TYPE
>>> + && same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
>>> {
>>> - /* The type of the array elements must be the same. */
>>> - if (!same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
>>> - return 0;
>>> -
>>> tree n1 = nelts_initialized_by_list_init (t1);
>>> tree n2 = nelts_initialized_by_list_init (t2);
>>> if (tree_int_cst_lt (n1, n2))
>>> diff --git a/gcc/testsuite/g++.dg/pr104996.C
>>> b/gcc/testsuite/g++.dg/pr104996.C
>>> new file mode 100644
>>> index 0000000000000..2e7558c7b9c77
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/pr104996.C
>>
>> ...can you please rename the test to initlist129.C and put it into
>> g++.dg/cpp0x?
>>
>>> @@ -0,0 +1,5 @@
>>
>> Also please add
>>
>> // PR c++/104996
>>
>> in the first line.
>>
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +template<unsigned size> char f(int (&&)[size]);
>>> +template<unsigned size> int f(int const (&)[size]);
>>> +static_assert(sizeof(f({1, 2, 3})) == 1, "");
>>>
>>
>> Marek
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] c++: Fall through for arrays of T vs T cv [PR104996]
2022-04-12 14:32 ` Jason Merrill
@ 2022-04-18 22:09 ` Ed Catmur
2022-04-21 12:48 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Ed Catmur @ 2022-04-18 22:09 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, Marek Polacek
If two arrays do not have the exact same element type including qualification, this could be e.g. f(int (&&)[]) vs. f(int const (&)[]), which can still be distinguished by the lvalue-rvalue tiebreaker.
By tightening this branch (in accordance with the letter of the Standard) we fall through to the next branch, which tests whether they have different element type ignoring qualification and returns 0 in that case; thus we only actually fall through in the T[...] vs. T cv[...] case, eventually considering the lvalue-rvalue tiebreaker at the end of compare_ics.
Bootstrapped and tested on x86_64-pc-linux-gnu.
Signed-off-by: Ed Catmur <ed@catmur.uk>
PR c++/104996
gcc/cp/ChangeLog:
* call.cc (compare_ics): When comparing list-initialization sequences, do not return early.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/initlist129.C: New test.
---
gcc/cp/call.cc | 7 ++-----
gcc/testsuite/g++.dg/cpp0x/initlist129.C | 6 ++++++
2 files changed, 8 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist129.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 51d8f6c3fb1..fa18d7f8f9d 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -11546,12 +11546,9 @@ compare_ics (conversion *ics1, conversion *ics2)
P0388R4.) */
else if (t1->kind == ck_aggr
&& TREE_CODE (t1->type) == ARRAY_TYPE
- && TREE_CODE (t2->type) == ARRAY_TYPE)
+ && TREE_CODE (t2->type) == ARRAY_TYPE
+ && same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
{
- /* The type of the array elements must be the same. */
- if (!same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
- return 0;
-
tree n1 = nelts_initialized_by_list_init (t1);
tree n2 = nelts_initialized_by_list_init (t2);
if (tree_int_cst_lt (n1, n2))
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist129.C b/gcc/testsuite/g++.dg/cpp0x/initlist129.C
new file mode 100644
index 00000000000..4d4faa9e08d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist129.C
@@ -0,0 +1,6 @@
+// PR c++/104996
+// { dg-do compile { target c++11 } }
+
+template<unsigned size> char f(int (&&)[size]);
+template<unsigned size> int f(int const (&)[size]);
+static_assert(sizeof(f({1, 2, 3})) == 1, "");
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Fall through for arrays of T vs T cv [PR104996]
2022-04-18 22:09 ` [PATCH v2] " Ed Catmur
@ 2022-04-21 12:48 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2022-04-21 12:48 UTC (permalink / raw)
To: Ed Catmur; +Cc: gcc-patches, Marek Polacek
On 4/18/22 18:09, Ed Catmur wrote:
> If two arrays do not have the exact same element type including qualification, this could be e.g. f(int (&&)[]) vs. f(int const (&)[]), which can still be distinguished by the lvalue-rvalue tiebreaker.
>
> By tightening this branch (in accordance with the letter of the Standard) we fall through to the next branch, which tests whether they have different element type ignoring qualification and returns 0 in that case; thus we only actually fall through in the T[...] vs. T cv[...] case, eventually considering the lvalue-rvalue tiebreaker at the end of compare_ics.
>
> Bootstrapped and tested on x86_64-pc-linux-gnu.
>
> Signed-off-by: Ed Catmur <ed@catmur.uk>
Applied, thanks.
> PR c++/104996
>
> gcc/cp/ChangeLog:
>
> * call.cc (compare_ics): When comparing list-initialization sequences, do not return early.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/initlist129.C: New test.
> ---
> gcc/cp/call.cc | 7 ++-----
> gcc/testsuite/g++.dg/cpp0x/initlist129.C | 6 ++++++
> 2 files changed, 8 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist129.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 51d8f6c3fb1..fa18d7f8f9d 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -11546,12 +11546,9 @@ compare_ics (conversion *ics1, conversion *ics2)
> P0388R4.) */
> else if (t1->kind == ck_aggr
> && TREE_CODE (t1->type) == ARRAY_TYPE
> - && TREE_CODE (t2->type) == ARRAY_TYPE)
> + && TREE_CODE (t2->type) == ARRAY_TYPE
> + && same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
> {
> - /* The type of the array elements must be the same. */
> - if (!same_type_p (TREE_TYPE (t1->type), TREE_TYPE (t2->type)))
> - return 0;
> -
> tree n1 = nelts_initialized_by_list_init (t1);
> tree n2 = nelts_initialized_by_list_init (t2);
> if (tree_int_cst_lt (n1, n2))
> diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist129.C b/gcc/testsuite/g++.dg/cpp0x/initlist129.C
> new file mode 100644
> index 00000000000..4d4faa9e08d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/initlist129.C
> @@ -0,0 +1,6 @@
> +// PR c++/104996
> +// { dg-do compile { target c++11 } }
> +
> +template<unsigned size> char f(int (&&)[size]);
> +template<unsigned size> int f(int const (&)[size]);
> +static_assert(sizeof(f({1, 2, 3})) == 1, "");
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-21 12:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-22 20:39 [PATCH] c++: Fall through for arrays of T vs T cv [PR104996] Ed Catmur
2022-03-22 20:59 ` Marek Polacek
2022-03-24 21:06 ` Jason Merrill
2022-04-12 14:32 ` Jason Merrill
2022-04-18 22:09 ` [PATCH v2] " Ed Catmur
2022-04-21 12:48 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).