diff mbox series

Workaround array_slice constructor portability issues (with older g++).

Message ID 006601da263f$d455aa30$7d00fe90$@nextmovesoftware.com
State New
Headers
Series Workaround array_slice constructor portability issues (with older g++). |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Roger Sayle Dec. 3, 2023, 11:24 p.m. UTC 
  The recent change to represent language and target attribute tables using
vec.h's array_slice template class triggers an issue/bug in older g++
compilers, specifically the g++ 4.8.5 system compiler of older RedHat
distributions.  This exhibits as the following compilation errors during
bootstrap:

../../gcc/gcc/c/c-lang.cc:55:2661: error: could not convert '(const
scoped_attribute_specs* const*)(& c_objc_attribute_table)' from 'const
scoped_attribute_specs* const*' to 'array_slice<const
scoped_attribute_specs* const>'
 struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;

../../gcc/gcc/c/c-decl.cc:4657:1: error: could not convert '(const
attribute_spec*)(& std_attributes)' from 'const attribute_spec*' to
'array_slice<const attribute_spec>'

Here the issue is with constructors of the from:

static const int table[] = { 1, 2, 3 };
array_slice<int> t = table;

Perhaps there's a fix possible in vec.h (an additional constructor?), but
the patch below fixes this issue by using one of array_slice's constructors
(that takes a size) explicitly, rather than rely on template resolution.
In the example above this looks like:

array_slice<int> t (table, 3);

or equivalently

array_slice<int> t = array_slice<int>(table, 3);

or equivalently

array_slice<int> t = array_slice<int>(table, ARRAY_SIZE (table));


This patch has been tested on x86_64-pc-linux-gnu with make bootstrap,
where these changes allow the bootstrap to complete.  Ok for mainline?
This fix might not by ideal, but it both draws attention to the problem
and restores bootstrap whilst better approaches are investigated.  For
example, an ARRAY_SLICE(table) macro might be appropriate if there isn't
an easy/portable template resolution solution.  Thoughts?


2023-12-03  Roger Sayle  <roger@nextmovesoftware.com>

gcc/c-family/ChangeLog
        * c-attribs.cc (c_common_gnu_attribute_table): Use an explicit
        array_slice constructor with an explicit size argument.
        (c_common_format_attribute_table): Likewise.

gcc/c/ChangeLog
        * c-decl.cc (std_attribute_table): Use an explicit
        array_slice constructor with an explicit size argument.
        * c-objc-common.h (LANG_HOOKS_ATTRIBUTE_TABLE): Likewise.

gcc/ChangeLog
        * config/i386/i386-options.cc (ix86_gnu_attribute_table): Use an
        explicit array_slice constructor with an explicit size argument.
        * config/i386/i386.cc (TARGET_ATTRIBUTE_TABLE): Likewise.

gcc/cp/ChangeLog
        * cp-objcp-common.h (LANG_HOOKS_ATTRIBUTE_TABLE): Use an
        explicit array_slice constructor with an explicit size argument.
        * tree.cc (cxx_gnu_attribute_table): Likewise.
        (std_attribute_table): Likewise.

gcc/lto/ChangeLog
        * lto-lang.cc (lto_gnu_attribute_table): Use an explicit
        array_slice constructor with an explicit size argument.
        (lto_format_attribute_table): Likewise.
        (LANG_HOOKS_ATTRIBUTE_TABLE): Likewise.


Thanks in advance,
Roger
--

Comments

Richard Sandiford Dec. 4, 2023, 10:41 a.m. UTC | #1 
"Roger Sayle" <roger@nextmovesoftware.com> writes:
> The recent change to represent language and target attribute tables using
> vec.h's array_slice template class triggers an issue/bug in older g++
> compilers, specifically the g++ 4.8.5 system compiler of older RedHat
> distributions.  This exhibits as the following compilation errors during
> bootstrap:
>
> ../../gcc/gcc/c/c-lang.cc:55:2661: error: could not convert '(const
> scoped_attribute_specs* const*)(& c_objc_attribute_table)' from 'const
> scoped_attribute_specs* const*' to 'array_slice<const
> scoped_attribute_specs* const>'
>  struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>
> ../../gcc/gcc/c/c-decl.cc:4657:1: error: could not convert '(const
> attribute_spec*)(& std_attributes)' from 'const attribute_spec*' to
> 'array_slice<const attribute_spec>'
>
> Here the issue is with constructors of the from:
>
> static const int table[] = { 1, 2, 3 };
> array_slice<int> t = table;

It's array_slice<const int> rather than array_slice<int>.  The above
would be invalid even with functioning compilers.

> Perhaps there's a fix possible in vec.h (an additional constructor?), but
> the patch below fixes this issue by using one of array_slice's constructors
> (that takes a size) explicitly, rather than rely on template resolution.
> In the example above this looks like:
>
> array_slice<int> t (table, 3);
>
> or equivalently
>
> array_slice<int> t = array_slice<int>(table, 3);
>
> or equivalently
>
> array_slice<int> t = array_slice<int>(table, ARRAY_SIZE (table));

Taking c-decl.cc as an arbitrary example, it seems to be enough to change:

const scoped_attribute_specs std_attribute_table =
{
  nullptr, std_attributes
};

to:

const scoped_attribute_specs std_attribute_table =
{
  nullptr, { std_attributes }
};

which seems less ugly than the explicit constructors.

But if we're going to do this, we should do it across the board,
not just for x86.

I think it's getting a bit ridiculous though.  Let's just accept
that 4.8.5 is not a fully functioning C++11 compiler and move on.
People who are still using that as their host compiler will need
to upgrade soon anyway, so we're just putting off the inevitable.
It's unlikely that these workarounds that we keep adding will ever
fully be removed.

Thanks,
Richard

> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap,
> where these changes allow the bootstrap to complete.  Ok for mainline?
> This fix might not by ideal, but it both draws attention to the problem
> and restores bootstrap whilst better approaches are investigated.  For
> example, an ARRAY_SLICE(table) macro might be appropriate if there isn't
> an easy/portable template resolution solution.  Thoughts?
>
>
> 2023-12-03  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/c-family/ChangeLog
>         * c-attribs.cc (c_common_gnu_attribute_table): Use an explicit
>         array_slice constructor with an explicit size argument.
>         (c_common_format_attribute_table): Likewise.
>
> gcc/c/ChangeLog
>         * c-decl.cc (std_attribute_table): Use an explicit
>         array_slice constructor with an explicit size argument.
>         * c-objc-common.h (LANG_HOOKS_ATTRIBUTE_TABLE): Likewise.
>
> gcc/ChangeLog
>         * config/i386/i386-options.cc (ix86_gnu_attribute_table): Use an
>         explicit array_slice constructor with an explicit size argument.
>         * config/i386/i386.cc (TARGET_ATTRIBUTE_TABLE): Likewise.
>
> gcc/cp/ChangeLog
>         * cp-objcp-common.h (LANG_HOOKS_ATTRIBUTE_TABLE): Use an
>         explicit array_slice constructor with an explicit size argument.
>         * tree.cc (cxx_gnu_attribute_table): Likewise.
>         (std_attribute_table): Likewise.
>
> gcc/lto/ChangeLog
>         * lto-lang.cc (lto_gnu_attribute_table): Use an explicit
>         array_slice constructor with an explicit size argument.
>         (lto_format_attribute_table): Likewise.
>         (LANG_HOOKS_ATTRIBUTE_TABLE): Likewise.
>
>
> Thanks in advance,
> Roger
> --
diff mbox series

Patch

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 45af074..af83588 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -584,7 +584,9 @@  const struct attribute_spec c_common_gnu_attributes[] =
 
 const struct scoped_attribute_specs c_common_gnu_attribute_table =
 {
-  "gnu", c_common_gnu_attributes
+  "gnu",
+  array_slice<const attribute_spec>(c_common_gnu_attributes,
+				    ARRAY_SIZE (c_common_gnu_attributes))
 };
 
 /* Give the specifications for the format attributes, used by C and all
@@ -603,7 +605,9 @@  const struct attribute_spec c_common_format_attributes[] =
 
 const struct scoped_attribute_specs c_common_format_attribute_table =
 {
-  "gnu", c_common_format_attributes
+  "gnu",
+  array_slice<const attribute_spec>(c_common_format_attributes,
+				    ARRAY_SIZE (c_common_format_attributes))
 };
 
 /* Returns TRUE iff the attribute indicated by ATTR_ID takes a plain
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 248d1bb..a6984b0 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -4653,7 +4653,8 @@  static const attribute_spec std_attributes[] =
 
 const scoped_attribute_specs std_attribute_table =
 {
-  nullptr, std_attributes
+  nullptr, array_slice<const attribute_spec>(std_attributes,
+					     ARRAY_SIZE (std_attributes))
 };
 
 /* Create the predefined scalar types of C,
diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h
index 426d938..021c651 100644
--- a/gcc/c/c-objc-common.h
+++ b/gcc/c/c-objc-common.h
@@ -83,7 +83,8 @@  static const scoped_attribute_specs *const c_objc_attribute_table[] =
 };
 
 #undef LANG_HOOKS_ATTRIBUTE_TABLE
-#define LANG_HOOKS_ATTRIBUTE_TABLE c_objc_attribute_table
+#define LANG_HOOKS_ATTRIBUTE_TABLE \
+array_slice<const scoped_attribute_specs* const> (c_objc_attribute_table, ARRAY_SIZE (c_objc_attribute_table))
 
 #undef LANG_HOOKS_TREE_DUMP_DUMP_TREE_FN
 #define LANG_HOOKS_TREE_DUMP_DUMP_TREE_FN c_dump_tree
diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
index 8776592..50b3425 100644
--- a/gcc/config/i386/i386-options.cc
+++ b/gcc/config/i386/i386-options.cc
@@ -4171,7 +4171,9 @@  static const attribute_spec ix86_gnu_attributes[] =
 
 const scoped_attribute_specs ix86_gnu_attribute_table =
 {
-  "gnu", ix86_gnu_attributes
+  "gnu",
+  array_slice<const attribute_spec>(ix86_gnu_attributes,
+				    ARRAY_SIZE (ix86_gnu_attributes))
 };
 
 #include "gt-i386-options.h"
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 0f91ee7..400b7cb 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -25981,7 +25981,7 @@  static const scoped_attribute_specs *const ix86_attribute_table[] =
 #define TARGET_LEGITIMIZE_ADDRESS ix86_legitimize_address
 
 #undef TARGET_ATTRIBUTE_TABLE
-#define TARGET_ATTRIBUTE_TABLE ix86_attribute_table
+#define TARGET_ATTRIBUTE_TABLE array_slice<const scoped_attribute_specs* const>(ix86_attribute_table, ARRAY_SIZE (ix86_attribute_table))
 #undef TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
 #define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P hook_bool_const_tree_true
 #if TARGET_DLLIMPORT_DECL_ATTRIBUTES
diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h
index b53d11e..a8d12a5 100644
--- a/gcc/cp/cp-objcp-common.h
+++ b/gcc/cp/cp-objcp-common.h
@@ -132,7 +132,7 @@  static const scoped_attribute_specs *const cp_objcp_attribute_table[] =
 };
 
 #undef LANG_HOOKS_ATTRIBUTE_TABLE
-#define LANG_HOOKS_ATTRIBUTE_TABLE cp_objcp_attribute_table
+#define LANG_HOOKS_ATTRIBUTE_TABLE array_slice<const scoped_attribute_specs* const> (cp_objcp_attribute_table, ARRAY_SIZE (cp_objcp_attribute_table))
 
 #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P
 #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P cp_var_mod_type_p
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index e0b9d51..989f36b 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -5098,7 +5098,9 @@  static const attribute_spec cxx_gnu_attributes[] =
 
 const scoped_attribute_specs cxx_gnu_attribute_table =
 {
-  "gnu", cxx_gnu_attributes
+  "gnu",
+  array_slice<const attribute_spec> (cxx_gnu_attributes,
+				     ARRAY_SIZE (cxx_gnu_attributes))
 };
 
 /* Table of C++ standard attributes.  */
@@ -5126,7 +5128,12 @@  static const attribute_spec std_attributes[] =
     handle_contract_attribute, NULL }
 };
 
-const scoped_attribute_specs std_attribute_table = { nullptr, std_attributes };
+const scoped_attribute_specs std_attribute_table =
+{
+  nullptr,
+  array_slice<const attribute_spec> (std_attributes,
+				     ARRAY_SIZE (std_attributes))
+};
 
 /* Handle an "init_priority" attribute; arguments as in
    struct attribute_spec.handler.  */
diff --git a/gcc/lto/lto-lang.cc b/gcc/lto/lto-lang.cc
index 41de35a..67cea40 100644
--- a/gcc/lto/lto-lang.cc
+++ b/gcc/lto/lto-lang.cc
@@ -140,7 +140,9 @@  static const attribute_spec lto_gnu_attributes[] =
 
 static const scoped_attribute_specs lto_gnu_attribute_table =
 {
-  "gnu", lto_gnu_attributes
+  "gnu",
+  array_slice<const attribute_spec>(lto_gnu_attributes,
+				    ARRAY_SIZE (lto_gnu_attributes))
 };
 
 /* Give the specifications for the format attributes, used by C and all
@@ -158,7 +160,9 @@  static const attribute_spec lto_format_attributes[] =
 
 static const scoped_attribute_specs lto_format_attribute_table =
 {
-  "gnu", lto_format_attributes
+  "gnu",
+  array_slice<const attribute_spec>(lto_format_attributes,
+				    ARRAY_SIZE (lto_format_attributes))
 };
 
 static const scoped_attribute_specs *const lto_attribute_table[] =
@@ -1478,7 +1482,7 @@  static void lto_init_ts (void)
 
 /* Attribute hooks.  */
 #undef LANG_HOOKS_ATTRIBUTE_TABLE
-#define LANG_HOOKS_ATTRIBUTE_TABLE lto_attribute_table
+#define LANG_HOOKS_ATTRIBUTE_TABLE array_slice<const scoped_attribute_specs* const> (lto_attribute_table, ARRAY_SIZE (lto_attribute_table))
 
 #undef LANG_HOOKS_BEGIN_SECTION
 #define LANG_HOOKS_BEGIN_SECTION lto_obj_begin_section