[13,RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]
Checks
Commit Message
When I was backporting the earlier 108099 patch I finally saw your comments on
the PR about the meaning of this pattern with the patch being wrong (and a
regression from 11). This fixes that regression; fixing the broader issues can
wait.
Tested x86_64-pc-linux-gnu, OK for 13.1 or wait for 13.2?
-- 8< --
My earlier patch for 108099 made us accept this non-standard pattern but
messed up the semantics, so that e.g. unsigned __int128_t was not a 128-bit
type.
PR c++/108099
gcc/cp/ChangeLog:
* decl.cc (grokdeclarator): Keep typedef_decl for __int128_t.
gcc/testsuite/ChangeLog:
* g++.dg/ext/int128-8.C: New test.
---
gcc/cp/decl.cc | 6 ++++--
gcc/testsuite/g++.dg/ext/int128-8.C | 24 ++++++++++++++++++++++++
2 files changed, 28 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/int128-8.C
base-commit: 57aecdbc118d4c1900d651cb3ada2c9632a67ad0
Comments
On Wed, Apr 19, 2023 at 11:20:09AM -0400, Jason Merrill wrote:
> When I was backporting the earlier 108099 patch I finally saw your comments on
> the PR about the meaning of this pattern with the patch being wrong (and a
> regression from 11). This fixes that regression; fixing the broader issues can
> wait.
>
> Tested x86_64-pc-linux-gnu, OK for 13.1 or wait for 13.2?
I'd wait for 13.2. We've been burned with trying to rush stuff out at the
last minute once this week already ;)
> -- 8< --
> My earlier patch for 108099 made us accept this non-standard pattern but
> messed up the semantics, so that e.g. unsigned __int128_t was not a 128-bit
> type.
>
> PR c++/108099
>
> gcc/cp/ChangeLog:
>
> * decl.cc (grokdeclarator): Keep typedef_decl for __int128_t.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/ext/int128-8.C: New test.
Jakub
On 4/19/23 11:26, Jakub Jelinek wrote:
> On Wed, Apr 19, 2023 at 11:20:09AM -0400, Jason Merrill wrote:
>> When I was backporting the earlier 108099 patch I finally saw your comments on
>> the PR about the meaning of this pattern with the patch being wrong (and a
>> regression from 11). This fixes that regression; fixing the broader issues can
>> wait.
>>
>> Tested x86_64-pc-linux-gnu, OK for 13.1 or wait for 13.2?
>
> I'd wait for 13.2. We've been burned with trying to rush stuff out at the
> last minute once this week already ;)
Fair, though this is much more straightforward than that issue.
I might revert the previous patch in that case, though; a wrong-code
regression seems worse than an ICE.
>> -- 8< --
>> My earlier patch for 108099 made us accept this non-standard pattern but
>> messed up the semantics, so that e.g. unsigned __int128_t was not a 128-bit
>> type.
>>
>> PR c++/108099
>>
>> gcc/cp/ChangeLog:
>>
>> * decl.cc (grokdeclarator): Keep typedef_decl for __int128_t.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * g++.dg/ext/int128-8.C: New test.
>
> Jakub
>
On Wed, Apr 19, 2023 at 12:48:42PM -0400, Jason Merrill wrote:
> On 4/19/23 11:26, Jakub Jelinek wrote:
> > On Wed, Apr 19, 2023 at 11:20:09AM -0400, Jason Merrill wrote:
> > > When I was backporting the earlier 108099 patch I finally saw your comments on
> > > the PR about the meaning of this pattern with the patch being wrong (and a
> > > regression from 11). This fixes that regression; fixing the broader issues can
> > > wait.
> > >
> > > Tested x86_64-pc-linux-gnu, OK for 13.1 or wait for 13.2?
> >
> > I'd wait for 13.2. We've been burned with trying to rush stuff out at the
> > last minute once this week already ;)
>
> Fair, though this is much more straightforward than that issue.
>
> I might revert the previous patch in that case, though; a wrong-code
> regression seems worse than an ICE.
It is wrong code on invalid source that we happen to tollerate, we don't
even know if it is from some real-world code or just some bad reduction.
And I believe in that area other cases just do something that user wouldn't
expect, so I wouldn't worry much about this particular PR for 13.1.
Jakub
On Wed, Apr 19, 2023 at 11:20:09AM -0400, Jason Merrill wrote:
> * g++.dg/ext/int128-8.C: New test.
The testcase needs to be restricted to int128 effective targets,
it expectedly fails on i386 and other 32-bit targets.
Tested using
GXX_TESTSUITE_STDS=98,11,14,17,20,2b make check-g++ RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} dg.exp=int128*.C'
on x86_64-linux before/after, committed to trunk and cherry-picked to
12 branch as obvious.
2023-04-20 Jakub Jelinek <jakub@redhat.com>
PR c++/108099
PR testsuite/109560
* g++.dg/ext/int128-8.C: Require int128 effective target.
--- gcc/testsuite/g++.dg/ext/int128-8.C.jj 2023-04-20 09:36:09.106375587 +0200
+++ gcc/testsuite/g++.dg/ext/int128-8.C 2023-04-20 09:37:02.429592525 +0200
@@ -1,5 +1,5 @@
// PR c++/108099
-// { dg-do compile { target c++11 } }
+// { dg-do compile { target { c++11 && int128 } } }
// { dg-options "" }
using u128 = unsigned __int128_t;
Jakub
@@ -12482,12 +12482,14 @@ grokdeclarator (const cp_declarator *declarator,
key, typedef_decl);
ok = !flag_pedantic_errors;
if (is_typedef_decl (typedef_decl))
- type = DECL_ORIGINAL_TYPE (typedef_decl);
+ {
+ type = DECL_ORIGINAL_TYPE (typedef_decl);
+ typedef_decl = NULL_TREE;
+ }
else
/* PR108099: __int128_t comes from c_common_nodes_and_builtins,
and is not built as a typedef. */
type = TREE_TYPE (typedef_decl);
- typedef_decl = NULL_TREE;
}
else if (declspecs->decltype_p)
error_at (loc, "%qs specified with %<decltype%>", key);
new file mode 100644
@@ -0,0 +1,24 @@
+// PR c++/108099
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+using u128 = unsigned __int128_t;
+using s128 = signed __int128_t;
+template <typename T, T v> struct integral_constant {
+ static constexpr T value = v;
+};
+typedef integral_constant <bool, false> false_type;
+typedef integral_constant <bool, true> true_type;
+template <class T, class U>
+struct is_same : false_type {};
+template <class T>
+struct is_same <T, T> : true_type {};
+static_assert (is_same <__int128, s128>::value, "");
+static_assert (is_same <signed __int128, s128>::value, "");
+static_assert (is_same <__int128_t, s128>::value, "");
+static_assert (is_same <unsigned __int128, u128>::value, ""); // { dg-bogus "" "" { xfail *-*-* } }
+static_assert (is_same <__uint128_t, u128>::value, ""); // { dg-bogus "" "" { xfail *-*-* } }
+static_assert (sizeof (s128) == sizeof (__int128), "");
+static_assert (sizeof (u128) == sizeof (unsigned __int128), "");
+static_assert (s128(-1) < 0, "");
+static_assert (u128(-1) > 0, "");