[13 PATCH RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]

[13 PATCH RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]

[Jason Merrill]

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

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, "");

base-commit: 57aecdbc118d4c1900d651cb3ada2c9632a67ad0
-- 
2.31.1

Re: [13 PATCH RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]

[Jakub Jelinek]

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

Re: [13 PATCH RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]

[Jason Merrill]

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
> 

Re: [13 PATCH RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]

[Jakub Jelinek]

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

Re: [13 PATCH RFA] c++: fix 'unsigned __int128_t' semantics [PR108099]

[Jakub Jelinek]

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