| Message ID | 20230419152009.494469-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [13,RFA] c++: fix 'unsigned __int128_t' semantics [PR108099] | expand |
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
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 772c059dc2c..ab5cb69b2ae 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -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); diff --git a/gcc/testsuite/g++.dg/ext/int128-8.C b/gcc/testsuite/g++.dg/ext/int128-8.C new file mode 100644 index 00000000000..14bbc49f5c3 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/int128-8.C @@ -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, "");