public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Fix ICE on defaulted spaceship with pointer return type [PR94162]
@ 2021-08-10  8:39 Jakub Jelinek
  2021-08-11 13:42 ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2021-08-10  8:39 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

The spaceship-synth-neg6.C testcase ICEs because we call cat_tag_for
on the explicit return type, but pointer types don't have
TYPE_LINKAGE_IDENTIFIER.  The patch fixes that.
Or should I be checking for if (!CLASS_TYPE_P (type)) return cc_last;
instead (are class type guaranteed to have TYPE_LINKAGE_IDENTIFIER?)?
I also wonder if after finding a match we shouldn't verify if is
a class type in std namespace (i.e. that
TYPE_NAME (TYPE_MAIN_VARIANT (type)) is a TYPE_DECL and
decl_in_std_namespace_p (TYPE_NAME (TYPE_MAIN_VARIANT (type)))
because it seems nothing prevents it from returning non-cc_last say on
namespace N {
  struct partial_ordering {};
}
etc.

The g++.dg/cpp2a/spaceship-synth11.C testcase is from a PR that has been
fixed with r12-619-gfc178519771db508c03611cff4a1466cf67fce1d (but
not backported to 11).

Bootstrapped/regtested on x86_64-linux and i686-linux.

2021-08-10  Jakub Jelinek  <jakub@redhat.com>

gcc/cp/
	PR c++/94162
	* method.c (cat_tag_for): Return cc_last for types with no
	linkage identifier.
gcc/testsuite/
	PR c++/99429
	* g++.dg/cpp2a/spaceship-synth11.C: New test.

	PR c++/94162
	* g++.dg/cpp2a/spaceship-synth-neg6.C: New test.

--- gcc/cp/method.c.jj	2021-06-25 10:36:22.169019953 +0200
+++ gcc/cp/method.c	2021-08-09 12:26:38.590166006 +0200
@@ -1029,6 +1029,8 @@ is_cat (tree type, comp_cat_tag tag)
 static comp_cat_tag
 cat_tag_for (tree type)
 {
+  if (!TYPE_LINKAGE_IDENTIFIER (type))
+    return cc_last;
   for (int i = 0; i < cc_last; ++i)
     {
       comp_cat_tag tag = (comp_cat_tag)i;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C.jj	2021-08-09 12:28:58.748240310 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C	2021-08-09 12:29:44.023618250 +0200
@@ -0,0 +1,29 @@
+// PR c++/99429
+// { dg-do compile { target c++20 } }
+
+namespace std {
+struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
+
+template <unsigned long N>
+struct duration {
+  static constexpr const long period = N;
+  constexpr duration (void) = default;
+  constexpr duration (const duration& d) = default;
+  constexpr bool operator== (const duration& d) const = default;
+  constexpr bool operator<=> (const duration& d) const = default;
+  long _d;
+};
+
+using nanoseconds = duration<1>;
+using microseconds = duration<nanoseconds::period * 1000>;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C.jj	2021-08-09 12:31:47.411922957 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C	2021-08-09 12:35:26.995906403 +0200
@@ -0,0 +1,11 @@
+// PR c++/94162
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+
+struct S {
+  int a;			// { dg-error "three-way comparison of 'S::a' has type 'std::strong_ordering', which does not convert to 'int\\*'" }
+  int *operator<=>(const S&) const = default;
+};
+
+bool b = S{} < S{};		// { dg-error "use of deleted function 'constexpr int\\* S::operator<=>\\\(const S&\\\) const'" }

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Fix ICE on defaulted spaceship with pointer return type [PR94162]
  2021-08-10  8:39 [PATCH] c++: Fix ICE on defaulted spaceship with pointer return type [PR94162] Jakub Jelinek
@ 2021-08-11 13:42 ` Jason Merrill
  2021-08-11 14:01   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2021-08-11 13:42 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 8/10/21 4:39 AM, Jakub Jelinek wrote:
> Hi!
> 
> The spaceship-synth-neg6.C testcase ICEs because we call cat_tag_for
> on the explicit return type, but pointer types don't have
> TYPE_LINKAGE_IDENTIFIER.  The patch fixes that.
> Or should I be checking for if (!CLASS_TYPE_P (type)) return cc_last;
> instead (are class type guaranteed to have TYPE_LINKAGE_IDENTIFIER?)?

> I also wonder if after finding a match we shouldn't verify if is
> a class type in std namespace (i.e. that
> TYPE_NAME (TYPE_MAIN_VARIANT (type)) is a TYPE_DECL
> and
> decl_in_std_namespace_p (TYPE_NAME (TYPE_MAIN_VARIANT (type)))
> because it seems nothing prevents it from returning non-cc_last say on
> namespace N {
>    struct partial_ordering {};
> }
> etc.

Checking CLASS_TYPE_P && decl_in_std_namespace (TYPE_MAIN_DECL) before 
looking at the name makes sense to me.

> The g++.dg/cpp2a/spaceship-synth11.C testcase is from a PR that has been
> fixed with r12-619-gfc178519771db508c03611cff4a1466cf67fce1d (but
> not backported to 11).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2021-08-10  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/cp/
> 	PR c++/94162
> 	* method.c (cat_tag_for): Return cc_last for types with no
> 	linkage identifier.
> gcc/testsuite/
> 	PR c++/99429
> 	* g++.dg/cpp2a/spaceship-synth11.C: New test.
> 
> 	PR c++/94162
> 	* g++.dg/cpp2a/spaceship-synth-neg6.C: New test.
> 
> --- gcc/cp/method.c.jj	2021-06-25 10:36:22.169019953 +0200
> +++ gcc/cp/method.c	2021-08-09 12:26:38.590166006 +0200
> @@ -1029,6 +1029,8 @@ is_cat (tree type, comp_cat_tag tag)
>   static comp_cat_tag
>   cat_tag_for (tree type)
>   {
> +  if (!TYPE_LINKAGE_IDENTIFIER (type))
> +    return cc_last;
>     for (int i = 0; i < cc_last; ++i)
>       {
>         comp_cat_tag tag = (comp_cat_tag)i;
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C.jj	2021-08-09 12:28:58.748240310 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C	2021-08-09 12:29:44.023618250 +0200
> @@ -0,0 +1,29 @@
> +// PR c++/99429
> +// { dg-do compile { target c++20 } }
> +
> +namespace std {
> +struct strong_ordering {
> +  int _v;
> +  constexpr strong_ordering (int v) :_v(v) {}
> +  constexpr operator int (void) const { return _v; }
> +  static const strong_ordering less;
> +  static const strong_ordering equal;
> +  static const strong_ordering greater;
> +};
> +constexpr strong_ordering strong_ordering::less = -1;
> +constexpr strong_ordering strong_ordering::equal = 0;
> +constexpr strong_ordering strong_ordering::greater = 1;
> +}
> +
> +template <unsigned long N>
> +struct duration {
> +  static constexpr const long period = N;
> +  constexpr duration (void) = default;
> +  constexpr duration (const duration& d) = default;
> +  constexpr bool operator== (const duration& d) const = default;
> +  constexpr bool operator<=> (const duration& d) const = default;
> +  long _d;
> +};
> +
> +using nanoseconds = duration<1>;
> +using microseconds = duration<nanoseconds::period * 1000>;
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C.jj	2021-08-09 12:31:47.411922957 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C	2021-08-09 12:35:26.995906403 +0200
> @@ -0,0 +1,11 @@
> +// PR c++/94162
> +// { dg-do compile { target c++20 } }
> +
> +#include <compare>
> +
> +struct S {
> +  int a;			// { dg-error "three-way comparison of 'S::a' has type 'std::strong_ordering', which does not convert to 'int\\*'" }
> +  int *operator<=>(const S&) const = default;
> +};
> +
> +bool b = S{} < S{};		// { dg-error "use of deleted function 'constexpr int\\* S::operator<=>\\\(const S&\\\) const'" }
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Fix ICE on defaulted spaceship with pointer return type [PR94162]
  2021-08-11 13:42 ` Jason Merrill
@ 2021-08-11 14:01   ` Jakub Jelinek
  2021-08-11 17:43     ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2021-08-11 14:01 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Wed, Aug 11, 2021 at 09:42:56AM -0400, Jason Merrill wrote:
> Checking CLASS_TYPE_P && decl_in_std_namespace (TYPE_MAIN_DECL) before
> looking at the name makes sense to me.

CLASS_TYPE_P is cheap, but isn't decl_in_std_namespace, especially when
it needs to walk inline namespaces, better done only if we get a match, so
like below?

Though I can do it even in the first if if you think it is better...

2021-08-11  Jakub Jelinek  <jakub@redhat.com>

gcc/cp/
	PR c++/94162
	* method.c (cat_tag_for): Return cc_last for !CLASS_TYPE_P
	or for classes not in std namespace.
gcc/testsuite/
	PR c++/99429
	* g++.dg/cpp2a/spaceship-synth11.C: New test.

	PR c++/94162
	* g++.dg/cpp2a/spaceship-synth-neg6.C: New test.

--- gcc/cp/method.c.jj	2021-08-09 15:03:00.923206463 +0200
+++ gcc/cp/method.c	2021-08-11 15:52:27.157437691 +0200
@@ -1029,10 +1029,13 @@ is_cat (tree type, comp_cat_tag tag)
 static comp_cat_tag
 cat_tag_for (tree type)
 {
+  if (!CLASS_TYPE_P (type))
+    return cc_last;
   for (int i = 0; i < cc_last; ++i)
     {
       comp_cat_tag tag = (comp_cat_tag)i;
-      if (is_cat (type, tag))
+      if (is_cat (type, tag)
+	  && decl_in_std_namespace_p (TYPE_MAIN_DECL (type)))
 	return tag;
     }
   return cc_last;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C.jj	2021-08-11 15:49:05.267204333 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C	2021-08-11 15:49:05.267204333 +0200
@@ -0,0 +1,29 @@
+// PR c++/99429
+// { dg-do compile { target c++20 } }
+
+namespace std {
+struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
+
+template <unsigned long N>
+struct duration {
+  static constexpr const long period = N;
+  constexpr duration (void) = default;
+  constexpr duration (const duration& d) = default;
+  constexpr bool operator== (const duration& d) const = default;
+  constexpr bool operator<=> (const duration& d) const = default;
+  long _d;
+};
+
+using nanoseconds = duration<1>;
+using microseconds = duration<nanoseconds::period * 1000>;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C.jj	2021-08-11 15:49:05.268204320 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C	2021-08-11 15:49:05.268204320 +0200
@@ -0,0 +1,11 @@
+// PR c++/94162
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+
+struct S {
+  int a;			// { dg-error "three-way comparison of 'S::a' has type 'std::strong_ordering', which does not convert to 'int\\*'" }
+  int *operator<=>(const S&) const = default;
+};
+
+bool b = S{} < S{};		// { dg-error "use of deleted function 'constexpr int\\* S::operator<=>\\\(const S&\\\) const'" }

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Fix ICE on defaulted spaceship with pointer return type [PR94162]
  2021-08-11 14:01   ` Jakub Jelinek
@ 2021-08-11 17:43     ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2021-08-11 17:43 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 8/11/21 10:01 AM, Jakub Jelinek wrote:
> On Wed, Aug 11, 2021 at 09:42:56AM -0400, Jason Merrill wrote:
>> Checking CLASS_TYPE_P && decl_in_std_namespace (TYPE_MAIN_DECL) before
>> looking at the name makes sense to me.
> 
> CLASS_TYPE_P is cheap, but isn't decl_in_std_namespace, especially when
> it needs to walk inline namespaces, better done only if we get a match, so
> like below?
> 
> Though I can do it even in the first if if you think it is better...

Let's do it first; no need to micro-optimize this function.

OK with that change.

> 2021-08-11  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/cp/
> 	PR c++/94162
> 	* method.c (cat_tag_for): Return cc_last for !CLASS_TYPE_P
> 	or for classes not in std namespace.
> gcc/testsuite/
> 	PR c++/99429
> 	* g++.dg/cpp2a/spaceship-synth11.C: New test.
> 
> 	PR c++/94162
> 	* g++.dg/cpp2a/spaceship-synth-neg6.C: New test.
> 
> --- gcc/cp/method.c.jj	2021-08-09 15:03:00.923206463 +0200
> +++ gcc/cp/method.c	2021-08-11 15:52:27.157437691 +0200
> @@ -1029,10 +1029,13 @@ is_cat (tree type, comp_cat_tag tag)
>   static comp_cat_tag
>   cat_tag_for (tree type)
>   {
> +  if (!CLASS_TYPE_P (type))
> +    return cc_last;
>     for (int i = 0; i < cc_last; ++i)
>       {
>         comp_cat_tag tag = (comp_cat_tag)i;
> -      if (is_cat (type, tag))
> +      if (is_cat (type, tag)
> +	  && decl_in_std_namespace_p (TYPE_MAIN_DECL (type)))
>   	return tag;
>       }
>     return cc_last;
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C.jj	2021-08-11 15:49:05.267204333 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth11.C	2021-08-11 15:49:05.267204333 +0200
> @@ -0,0 +1,29 @@
> +// PR c++/99429
> +// { dg-do compile { target c++20 } }
> +
> +namespace std {
> +struct strong_ordering {
> +  int _v;
> +  constexpr strong_ordering (int v) :_v(v) {}
> +  constexpr operator int (void) const { return _v; }
> +  static const strong_ordering less;
> +  static const strong_ordering equal;
> +  static const strong_ordering greater;
> +};
> +constexpr strong_ordering strong_ordering::less = -1;
> +constexpr strong_ordering strong_ordering::equal = 0;
> +constexpr strong_ordering strong_ordering::greater = 1;
> +}
> +
> +template <unsigned long N>
> +struct duration {
> +  static constexpr const long period = N;
> +  constexpr duration (void) = default;
> +  constexpr duration (const duration& d) = default;
> +  constexpr bool operator== (const duration& d) const = default;
> +  constexpr bool operator<=> (const duration& d) const = default;
> +  long _d;
> +};
> +
> +using nanoseconds = duration<1>;
> +using microseconds = duration<nanoseconds::period * 1000>;
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C.jj	2021-08-11 15:49:05.268204320 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C	2021-08-11 15:49:05.268204320 +0200
> @@ -0,0 +1,11 @@
> +// PR c++/94162
> +// { dg-do compile { target c++20 } }
> +
> +#include <compare>
> +
> +struct S {
> +  int a;			// { dg-error "three-way comparison of 'S::a' has type 'std::strong_ordering', which does not convert to 'int\\*'" }
> +  int *operator<=>(const S&) const = default;
> +};
> +
> +bool b = S{} < S{};		// { dg-error "use of deleted function 'constexpr int\\* S::operator<=>\\\(const S&\\\) const'" }
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-11 17:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10  8:39 [PATCH] c++: Fix ICE on defaulted spaceship with pointer return type [PR94162] Jakub Jelinek
2021-08-11 13:42 ` Jason Merrill
2021-08-11 14:01   ` Jakub Jelinek
2021-08-11 17:43     ` 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).