public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] vec.h: Guard most of static assertions for GCC >= 5
@ 2023-09-29 10:42 Jakub Jelinek
  2023-09-29 12:40 ` Richard Biener
  2023-09-30 19:30 ` Patrick O'Neill
  0 siblings, 2 replies; 3+ messages in thread
From: Jakub Jelinek @ 2023-09-29 10:42 UTC (permalink / raw)
  To: Richard Biener, Jonathan Wakely; +Cc: gcc-patches

Hi!

As reported by Jonathan on IRC, my vec.h patch broke build with GCC 4.8.x
or 4.9.x as system compiler, e.g. on CFarm.
The problem is that while all of
std::is_trivially_{destructible,copyable,default_constructible} traits
are in C++, only std::is_trivially_destructible has been implemented in GCC
4.8, the latter two were added only in GCC 5.
Only std::is_trivially_destructible is the really important one though,
which is used to decide what pop returns and whether to invoke the
destructors or not.  The rest are solely used in static_asserts and as such
I think it is acceptable if we don't assert those when built with GCC 4.8
or 4.9, anybody doing bootstrap from those system compilers or doing builds
with newer GCC will catch that.

So, the following patch guards those for 5+.
If we switch to C++14 later on and start requiring newer version of system
GCC as well (do we require GCC >= 5 which claims the last C++14 language
features, or what provides all C++14 library features, or GCC >= 6 which
uses -std=c++14 by default?), this patch then can be reverted.

Ok for trunk?

2023-09-29  Jakub Jelinek  <jakub@redhat.com>

	* vec.h (quick_insert, ordered_remove, unordered_remove,
	block_remove, qsort, sort, stablesort, quick_grow): Guard
	std::is_trivially_{copyable,default_constructible} and
	vec_detail::is_trivially_copyable_or_pair static assertions
	with GCC_VERSION >= 5000.
	(vec_detail::is_trivially_copyable_or_pair): Guard definition
	with GCC_VERSION >= 5000.

--- gcc/vec.h.jj	2023-09-29 10:59:09.830551963 +0200
+++ gcc/vec.h	2023-09-29 12:29:32.676428677 +0200
@@ -1086,7 +1086,12 @@ vec<T, A, vl_embed>::quick_insert (unsig
 {
   gcc_checking_assert (length () < allocated ());
   gcc_checking_assert (ix <= length ());
+#if GCC_VERSION >= 5000
+  /* GCC 4.8 and 4.9 only implement std::is_trivially_destructible,
+     but not std::is_trivially_copyable nor
+     std::is_trivially_default_constructible.  */
   static_assert (std::is_trivially_copyable <T>::value, "");
+#endif
   T *slot = &address ()[ix];
   memmove (slot + 1, slot, (m_vecpfx.m_num++ - ix) * sizeof (T));
   *slot = obj;
@@ -1102,7 +1107,9 @@ inline void
 vec<T, A, vl_embed>::ordered_remove (unsigned ix)
 {
   gcc_checking_assert (ix < length ());
+#if GCC_VERSION >= 5000
   static_assert (std::is_trivially_copyable <T>::value, "");
+#endif
   T *slot = &address ()[ix];
   memmove (slot, slot + 1, (--m_vecpfx.m_num - ix) * sizeof (T));
 }
@@ -1150,7 +1157,9 @@ inline void
 vec<T, A, vl_embed>::unordered_remove (unsigned ix)
 {
   gcc_checking_assert (ix < length ());
+#if GCC_VERSION >= 5000
   static_assert (std::is_trivially_copyable <T>::value, "");
+#endif
   T *p = address ();
   p[ix] = p[--m_vecpfx.m_num];
 }
@@ -1164,13 +1173,16 @@ inline void
 vec<T, A, vl_embed>::block_remove (unsigned ix, unsigned len)
 {
   gcc_checking_assert (ix + len <= length ());
+#if GCC_VERSION >= 5000
   static_assert (std::is_trivially_copyable <T>::value, "");
+#endif
   T *slot = &address ()[ix];
   m_vecpfx.m_num -= len;
   memmove (slot, slot + len, (m_vecpfx.m_num - ix) * sizeof (T));
 }
 
 
+#if GCC_VERSION >= 5000
 namespace vec_detail
 {
   /* gcc_{qsort,qsort_r,stablesort_r} implementation under the hood
@@ -1189,6 +1201,7 @@ namespace vec_detail
   : std::integral_constant<bool, std::is_trivially_copyable<T>::value
     && std::is_trivially_copyable<U>::value> { };
 }
+#endif
 
 /* Sort the contents of this vector with qsort.  CMP is the comparison
    function to pass to qsort.  */
@@ -1197,7 +1210,9 @@ template<typename T, typename A>
 inline void
 vec<T, A, vl_embed>::qsort (int (*cmp) (const void *, const void *))
 {
+#if GCC_VERSION >= 5000
   static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
+#endif
   if (length () > 1)
     gcc_qsort (address (), length (), sizeof (T), cmp);
 }
@@ -1210,7 +1225,9 @@ inline void
 vec<T, A, vl_embed>::sort (int (*cmp) (const void *, const void *, void *),
 			   void *data)
 {
+#if GCC_VERSION >= 5000
   static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
+#endif
   if (length () > 1)
     gcc_sort_r (address (), length (), sizeof (T), cmp, data);
 }
@@ -1223,7 +1240,9 @@ inline void
 vec<T, A, vl_embed>::stablesort (int (*cmp) (const void *, const void *,
 					     void *), void *data)
 {
+#if GCC_VERSION >= 5000
   static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
+#endif
   if (length () > 1)
     gcc_stablesort_r (address (), length (), sizeof (T), cmp, data);
 }
@@ -1396,7 +1415,9 @@ inline void
 vec<T, A, vl_embed>::quick_grow (unsigned len)
 {
   gcc_checking_assert (length () <= len && len <= m_vecpfx.m_alloc);
+#if GCC_VERSION >= 5000
 //  static_assert (std::is_trivially_default_constructible <T>::value, "");
+#endif
   m_vecpfx.m_num = len;
 }
 

	Jakub


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

* Re: [PATCH] vec.h: Guard most of static assertions for GCC >= 5
  2023-09-29 10:42 [PATCH] vec.h: Guard most of static assertions for GCC >= 5 Jakub Jelinek
@ 2023-09-29 12:40 ` Richard Biener
  2023-09-30 19:30 ` Patrick O'Neill
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2023-09-29 12:40 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jonathan Wakely, gcc-patches

On Fri, 29 Sep 2023, Jakub Jelinek wrote:

> Hi!
> 
> As reported by Jonathan on IRC, my vec.h patch broke build with GCC 4.8.x
> or 4.9.x as system compiler, e.g. on CFarm.
> The problem is that while all of
> std::is_trivially_{destructible,copyable,default_constructible} traits
> are in C++, only std::is_trivially_destructible has been implemented in GCC
> 4.8, the latter two were added only in GCC 5.
> Only std::is_trivially_destructible is the really important one though,
> which is used to decide what pop returns and whether to invoke the
> destructors or not.  The rest are solely used in static_asserts and as such
> I think it is acceptable if we don't assert those when built with GCC 4.8
> or 4.9, anybody doing bootstrap from those system compilers or doing builds
> with newer GCC will catch that.
> 
> So, the following patch guards those for 5+.
> If we switch to C++14 later on and start requiring newer version of system
> GCC as well (do we require GCC >= 5 which claims the last C++14 language
> features, or what provides all C++14 library features, or GCC >= 6 which
> uses -std=c++14 by default?), this patch then can be reverted.

I'd say we can recommend a compiler with all language and library features
but "support" the oldest working compiler.  For us it's practically
GCC 4.8 (doesn't work) or GCC 7 (new enough anyway).

> Ok for trunk?

OK.

Richard.

> 2023-09-29  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* vec.h (quick_insert, ordered_remove, unordered_remove,
> 	block_remove, qsort, sort, stablesort, quick_grow): Guard
> 	std::is_trivially_{copyable,default_constructible} and
> 	vec_detail::is_trivially_copyable_or_pair static assertions
> 	with GCC_VERSION >= 5000.
> 	(vec_detail::is_trivially_copyable_or_pair): Guard definition
> 	with GCC_VERSION >= 5000.
> 
> --- gcc/vec.h.jj	2023-09-29 10:59:09.830551963 +0200
> +++ gcc/vec.h	2023-09-29 12:29:32.676428677 +0200
> @@ -1086,7 +1086,12 @@ vec<T, A, vl_embed>::quick_insert (unsig
>  {
>    gcc_checking_assert (length () < allocated ());
>    gcc_checking_assert (ix <= length ());
> +#if GCC_VERSION >= 5000
> +  /* GCC 4.8 and 4.9 only implement std::is_trivially_destructible,
> +     but not std::is_trivially_copyable nor
> +     std::is_trivially_default_constructible.  */
>    static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>    T *slot = &address ()[ix];
>    memmove (slot + 1, slot, (m_vecpfx.m_num++ - ix) * sizeof (T));
>    *slot = obj;
> @@ -1102,7 +1107,9 @@ inline void
>  vec<T, A, vl_embed>::ordered_remove (unsigned ix)
>  {
>    gcc_checking_assert (ix < length ());
> +#if GCC_VERSION >= 5000
>    static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>    T *slot = &address ()[ix];
>    memmove (slot, slot + 1, (--m_vecpfx.m_num - ix) * sizeof (T));
>  }
> @@ -1150,7 +1157,9 @@ inline void
>  vec<T, A, vl_embed>::unordered_remove (unsigned ix)
>  {
>    gcc_checking_assert (ix < length ());
> +#if GCC_VERSION >= 5000
>    static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>    T *p = address ();
>    p[ix] = p[--m_vecpfx.m_num];
>  }
> @@ -1164,13 +1173,16 @@ inline void
>  vec<T, A, vl_embed>::block_remove (unsigned ix, unsigned len)
>  {
>    gcc_checking_assert (ix + len <= length ());
> +#if GCC_VERSION >= 5000
>    static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>    T *slot = &address ()[ix];
>    m_vecpfx.m_num -= len;
>    memmove (slot, slot + len, (m_vecpfx.m_num - ix) * sizeof (T));
>  }
>  
>  
> +#if GCC_VERSION >= 5000
>  namespace vec_detail
>  {
>    /* gcc_{qsort,qsort_r,stablesort_r} implementation under the hood
> @@ -1189,6 +1201,7 @@ namespace vec_detail
>    : std::integral_constant<bool, std::is_trivially_copyable<T>::value
>      && std::is_trivially_copyable<U>::value> { };
>  }
> +#endif
>  
>  /* Sort the contents of this vector with qsort.  CMP is the comparison
>     function to pass to qsort.  */
> @@ -1197,7 +1210,9 @@ template<typename T, typename A>
>  inline void
>  vec<T, A, vl_embed>::qsort (int (*cmp) (const void *, const void *))
>  {
> +#if GCC_VERSION >= 5000
>    static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
> +#endif
>    if (length () > 1)
>      gcc_qsort (address (), length (), sizeof (T), cmp);
>  }
> @@ -1210,7 +1225,9 @@ inline void
>  vec<T, A, vl_embed>::sort (int (*cmp) (const void *, const void *, void *),
>  			   void *data)
>  {
> +#if GCC_VERSION >= 5000
>    static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
> +#endif
>    if (length () > 1)
>      gcc_sort_r (address (), length (), sizeof (T), cmp, data);
>  }
> @@ -1223,7 +1240,9 @@ inline void
>  vec<T, A, vl_embed>::stablesort (int (*cmp) (const void *, const void *,
>  					     void *), void *data)
>  {
> +#if GCC_VERSION >= 5000
>    static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
> +#endif
>    if (length () > 1)
>      gcc_stablesort_r (address (), length (), sizeof (T), cmp, data);
>  }
> @@ -1396,7 +1415,9 @@ inline void
>  vec<T, A, vl_embed>::quick_grow (unsigned len)
>  {
>    gcc_checking_assert (length () <= len && len <= m_vecpfx.m_alloc);
> +#if GCC_VERSION >= 5000
>  //  static_assert (std::is_trivially_default_constructible <T>::value, "");
> +#endif
>    m_vecpfx.m_num = len;
>  }
>  
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

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

* Re: [PATCH] vec.h: Guard most of static assertions for GCC >= 5
  2023-09-29 10:42 [PATCH] vec.h: Guard most of static assertions for GCC >= 5 Jakub Jelinek
  2023-09-29 12:40 ` Richard Biener
@ 2023-09-30 19:30 ` Patrick O'Neill
  1 sibling, 0 replies; 3+ messages in thread
From: Patrick O'Neill @ 2023-09-30 19:30 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, rguenther, jwakely

Hi Jakub,

A follow-up commit of yours (9d249b7e31e) is causing bootstrap failures 
for riscv*-*-* targets.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111649

Patrick

On 9/29/23 03:42, Jakub Jelinek wrote:
> Hi!
>
> As reported by Jonathan on IRC, my vec.h patch broke build with GCC 4.8.x
> or 4.9.x as system compiler, e.g. on CFarm.
> The problem is that while all of
> std::is_trivially_{destructible,copyable,default_constructible} traits
> are in C++, only std::is_trivially_destructible has been implemented in GCC
> 4.8, the latter two were added only in GCC 5.
> Only std::is_trivially_destructible is the really important one though,
> which is used to decide what pop returns and whether to invoke the
> destructors or not.  The rest are solely used in static_asserts and as such
> I think it is acceptable if we don't assert those when built with GCC 4.8
> or 4.9, anybody doing bootstrap from those system compilers or doing builds
> with newer GCC will catch that.
>
> So, the following patch guards those for 5+.
> If we switch to C++14 later on and start requiring newer version of system
> GCC as well (do we require GCC >= 5 which claims the last C++14 language
> features, or what provides all C++14 library features, or GCC >= 6 which
> uses -std=c++14 by default?), this patch then can be reverted.
>
> Ok for trunk?
>
> 2023-09-29  Jakub Jelinek  <jakub@redhat.com>
>
> 	* vec.h (quick_insert, ordered_remove, unordered_remove,
> 	block_remove, qsort, sort, stablesort, quick_grow): Guard
> 	std::is_trivially_{copyable,default_constructible} and
> 	vec_detail::is_trivially_copyable_or_pair static assertions
> 	with GCC_VERSION >= 5000.
> 	(vec_detail::is_trivially_copyable_or_pair): Guard definition
> 	with GCC_VERSION >= 5000.
>
> --- gcc/vec.h.jj	2023-09-29 10:59:09.830551963 +0200
> +++ gcc/vec.h	2023-09-29 12:29:32.676428677 +0200
> @@ -1086,7 +1086,12 @@ vec<T, A, vl_embed>::quick_insert (unsig
>   {
>     gcc_checking_assert (length () < allocated ());
>     gcc_checking_assert (ix <= length ());
> +#if GCC_VERSION >= 5000
> +  /* GCC 4.8 and 4.9 only implement std::is_trivially_destructible,
> +     but not std::is_trivially_copyable nor
> +     std::is_trivially_default_constructible.  */
>     static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>     T *slot = &address ()[ix];
>     memmove (slot + 1, slot, (m_vecpfx.m_num++ - ix) * sizeof (T));
>     *slot = obj;
> @@ -1102,7 +1107,9 @@ inline void
>   vec<T, A, vl_embed>::ordered_remove (unsigned ix)
>   {
>     gcc_checking_assert (ix < length ());
> +#if GCC_VERSION >= 5000
>     static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>     T *slot = &address ()[ix];
>     memmove (slot, slot + 1, (--m_vecpfx.m_num - ix) * sizeof (T));
>   }
> @@ -1150,7 +1157,9 @@ inline void
>   vec<T, A, vl_embed>::unordered_remove (unsigned ix)
>   {
>     gcc_checking_assert (ix < length ());
> +#if GCC_VERSION >= 5000
>     static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>     T *p = address ();
>     p[ix] = p[--m_vecpfx.m_num];
>   }
> @@ -1164,13 +1173,16 @@ inline void
>   vec<T, A, vl_embed>::block_remove (unsigned ix, unsigned len)
>   {
>     gcc_checking_assert (ix + len <= length ());
> +#if GCC_VERSION >= 5000
>     static_assert (std::is_trivially_copyable <T>::value, "");
> +#endif
>     T *slot = &address ()[ix];
>     m_vecpfx.m_num -= len;
>     memmove (slot, slot + len, (m_vecpfx.m_num - ix) * sizeof (T));
>   }
>   
>   
> +#if GCC_VERSION >= 5000
>   namespace vec_detail
>   {
>     /* gcc_{qsort,qsort_r,stablesort_r} implementation under the hood
> @@ -1189,6 +1201,7 @@ namespace vec_detail
>     : std::integral_constant<bool, std::is_trivially_copyable<T>::value
>       && std::is_trivially_copyable<U>::value> { };
>   }
> +#endif
>   
>   /* Sort the contents of this vector with qsort.  CMP is the comparison
>      function to pass to qsort.  */
> @@ -1197,7 +1210,9 @@ template<typename T, typename A>
>   inline void
>   vec<T, A, vl_embed>::qsort (int (*cmp) (const void *, const void *))
>   {
> +#if GCC_VERSION >= 5000
>     static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
> +#endif
>     if (length () > 1)
>       gcc_qsort (address (), length (), sizeof (T), cmp);
>   }
> @@ -1210,7 +1225,9 @@ inline void
>   vec<T, A, vl_embed>::sort (int (*cmp) (const void *, const void *, void *),
>   			   void *data)
>   {
> +#if GCC_VERSION >= 5000
>     static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
> +#endif
>     if (length () > 1)
>       gcc_sort_r (address (), length (), sizeof (T), cmp, data);
>   }
> @@ -1223,7 +1240,9 @@ inline void
>   vec<T, A, vl_embed>::stablesort (int (*cmp) (const void *, const void *,
>   					     void *), void *data)
>   {
> +#if GCC_VERSION >= 5000
>     static_assert (vec_detail::is_trivially_copyable_or_pair <T>::value, "");
> +#endif
>     if (length () > 1)
>       gcc_stablesort_r (address (), length (), sizeof (T), cmp, data);
>   }
> @@ -1396,7 +1415,9 @@ inline void
>   vec<T, A, vl_embed>::quick_grow (unsigned len)
>   {
>     gcc_checking_assert (length () <= len && len <= m_vecpfx.m_alloc);
> +#if GCC_VERSION >= 5000
>   //  static_assert (std::is_trivially_default_constructible <T>::value, "");
> +#endif
>     m_vecpfx.m_num = len;
>   }
>   
>
> 	Jakub
>

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

end of thread, other threads:[~2023-09-30 19:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-29 10:42 [PATCH] vec.h: Guard most of static assertions for GCC >= 5 Jakub Jelinek
2023-09-29 12:40 ` Richard Biener
2023-09-30 19:30 ` Patrick O'Neill

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).