[PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Roland McGrath]

The std::basic_string template type is only specified for
instantiations using character types.  Newer (LLVM) libc++
implementations no longer allow non-character integer types
to be used.

Ok for trunk?

Thanks,
Roland

diff --git a/gold/merge.cc b/gold/merge.cc
index c12efc9905e..ce31a792443 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -665,10 +665,10 @@ template
 class Output_merge_string<char>;

 template
-class Output_merge_string<uint16_t>;
+class Output_merge_string<char16_t>;

 template
-class Output_merge_string<uint32_t>;
+class Output_merge_string<char32_t>;

 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
 template
diff --git a/gold/output.cc b/gold/output.cc
index a1978eb5f32..6053e4db33d 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -29,6 +29,7 @@
 #include <unistd.h>
 #include <sys/stat.h>
 #include <algorithm>
+#include <uchar.h>

 #ifdef HAVE_SYS_MMAN_H
 #include <sys/mman.h>
@@ -2706,10 +2707,10 @@
Output_section::add_merge_input_section(Relobj* object, unsigned int
shndx,
              pomb = new Output_merge_string<char>(addralign);
              break;
            case 2:
-             pomb = new Output_merge_string<uint16_t>(addralign);
+             pomb = new Output_merge_string<char16_t>(addralign);
              break;
            case 4:
-             pomb = new Output_merge_string<uint32_t>(addralign);
+             pomb = new Output_merge_string<char32_t>(addralign);
              break;
            default:
              return false;
diff --git a/gold/stringpool.cc b/gold/stringpool.cc
index a2cd44d5244..b5ac1dd34ca 100644
--- a/gold/stringpool.cc
+++ b/gold/stringpool.cc
@@ -25,6 +25,7 @@
 #include <cstring>
 #include <algorithm>
 #include <vector>
+#include <uchar.h>

 #include "output.h"
 #include "parameters.h"
@@ -527,9 +528,9 @@ template
 class Stringpool_template<char>;

 template
-class Stringpool_template<uint16_t>;
+class Stringpool_template<char16_t>;

 template
-class Stringpool_template<uint32_t>;
+class Stringpool_template<char32_t>;

 } // End namespace gold.

Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Cary Coutant]

> The std::basic_string template type is only specified for
> instantiations using character types.  Newer (LLVM) libc++
> implementations no longer allow non-character integer types
> to be used.
>
> Ok for trunk?

LGTM. Thanks!

-cary

Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Roland McGrath]

[-- Attachment #1: Type: text/plain, Size: 27 bytes --]

Committed.

Thanks,
Roland

Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Vaseeharan Vinayagamoorthy]

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

Hello,


This patch has started causing errors in our builds, using GCC 4.8.5.
This is affecting our cross builds of these targets:
aarch64_be_none_linux_gnu
aarch64_none_linux_gnu
arm_none_linux_gnueabi
arm_none_linux_gnueabihf


The errors are:


/…/src/binutils-gdb/gold/merge.cc:668:27: error: ‘char16_t’ was not declared in this scope


 class Output_merge_string<char16_t>;


                           ^


/…/src/binutils-gdb/gold/merge.cc:668:35: error: template argument 1 is invalid


 class Output_merge_string<char16_t>;


                                   ^


/…/src/binutils-gdb/gold/merge.cc:671:27: error: ‘char32_t’ was not declared in this scope


 class Output_merge_string<char32_t>;


                           ^


/…/src/binutils-gdb/gold/merge.cc:671:35: error: template argument 1 is invalid


 class Output_merge_string<char32_t>;


                                   ^


Kind regards,
Vasee



________________________________
From: Binutils <binutils-bounces+vvinayag=arm.com@sourceware.org> on behalf of Roland McGrath via Binutils <binutils@sourceware.org>
Sent: 05 September 2023 20:35
To: Binutils <binutils@sourceware.org>
Subject: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

The std::basic_string template type is only specified for
instantiations using character types.  Newer (LLVM) libc++
implementations no longer allow non-character integer types
to be used.

Ok for trunk?

Thanks,
Roland

diff --git a/gold/merge.cc b/gold/merge.cc
index c12efc9905e..ce31a792443 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -665,10 +665,10 @@ template
 class Output_merge_string<char>;

 template
-class Output_merge_string<uint16_t>;
+class Output_merge_string<char16_t>;

 template
-class Output_merge_string<uint32_t>;
+class Output_merge_string<char32_t>;

 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
 template
diff --git a/gold/output.cc b/gold/output.cc
index a1978eb5f32..6053e4db33d 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -29,6 +29,7 @@
 #include <unistd.h>
 #include <sys/stat.h>
 #include <algorithm>
+#include <uchar.h>

 #ifdef HAVE_SYS_MMAN_H
 #include <sys/mman.h>
@@ -2706,10 +2707,10 @@
Output_section::add_merge_input_section(Relobj* object, unsigned int
shndx,
              pomb = new Output_merge_string<char>(addralign);
              break;
            case 2:
-             pomb = new Output_merge_string<uint16_t>(addralign);
+             pomb = new Output_merge_string<char16_t>(addralign);
              break;
            case 4:
-             pomb = new Output_merge_string<uint32_t>(addralign);
+             pomb = new Output_merge_string<char32_t>(addralign);
              break;
            default:
              return false;
diff --git a/gold/stringpool.cc b/gold/stringpool.cc
index a2cd44d5244..b5ac1dd34ca 100644
--- a/gold/stringpool.cc
+++ b/gold/stringpool.cc
@@ -25,6 +25,7 @@
 #include <cstring>
 #include <algorithm>
 #include <vector>
+#include <uchar.h>

 #include "output.h"
 #include "parameters.h"
@@ -527,9 +528,9 @@ template
 class Stringpool_template<char>;

 template
-class Stringpool_template<uint16_t>;
+class Stringpool_template<char16_t>;

 template
-class Stringpool_template<uint32_t>;
+class Stringpool_template<char32_t>;

 } // End namespace gold.

Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Roland McGrath]

I didn't realize that building gold with compilers so old they don't
support C++11 was supported.  The GCC source history suggests that
char16_t and char32_t support were introduced in GCC 4.4.  I'm not
sure when the default C++ mode was changed to be >= -std=c++11.  It
looks to me that the configure stuff should be making it pass
`-std=c++11` if that's required, but I don't know how that test is
coming out in your build.  I wonder if using `CXX=g++ -std=c++11`
makes a difference with your version.

Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Vaseeharan Vinayagamoorthy]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

Hi,

Sorry for the delayed response.
Using -std=gnu++11 in our build scripts when building binutils does fix the issue for us, as mentioned in https://sourceware.org/bugzilla/show_bug.cgi?id=30867 .
However, does this need fixing in configure.ac instead?

Kind regards,
Vasee
________________________________
From: Roland McGrath <mcgrathr@google.com>
Sent: 20 September 2023 01:41
To: Vaseeharan Vinayagamoorthy <Vaseeharan.Vinayagamoorthy@arm.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

I didn't realize that building gold with compilers so old they don't
support C++11 was supported.  The GCC source history suggests that
char16_t and char32_t support were introduced in GCC 4.4.  I'm not
sure when the default C++ mode was changed to be >= -std=c++11.  It
looks to me that the configure stuff should be making it pass
`-std=c++11` if that's required, but I don't know how that test is
coming out in your build.  I wonder if using `CXX=g++ -std=c++11`
makes a difference with your version.

Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types

[Roland McGrath]

The top-level configure already uses AX_CXX_COMPILE_STDCXX(11), which
should ensure `-std=c++11` is added if needed.  Perhaps either that
check is not having the result that it should in your configuration,
or the flags it sets are not being properly plumbed down to the
subdirectory configure/makefile flags.  For example, gdb/configure.ac
also does this itself and I'm not sure if that means that each
subdirectory configure.ac needs it as well. It seems likely that we
now need to change the invocation to be AX_CXX_COMPILE_STDCXX(11, ,
mandatory) as gdb/configure.ac does.

On Thu, Oct 19, 2023 at 4:08 AM Vaseeharan Vinayagamoorthy
<Vaseeharan.Vinayagamoorthy@arm.com> wrote:
>
> Hi,
>
> Sorry for the delayed response.
> Using -std=gnu++11 in our build scripts when building binutils does fix the issue for us, as mentioned in https://sourceware.org/bugzilla/show_bug.cgi?id=30867 .
> However, does this need fixing in configure.ac instead?
>
> Kind regards,
> Vasee
> ________________________________
> From: Roland McGrath <mcgrathr@google.com>
> Sent: 20 September 2023 01:41
> To: Vaseeharan Vinayagamoorthy <Vaseeharan.Vinayagamoorthy@arm.com>
> Cc: Binutils <binutils@sourceware.org>
> Subject: Re: [PATCH users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types
>
> I didn't realize that building gold with compilers so old they don't
> support C++11 was supported.  The GCC source history suggests that
> char16_t and char32_t support were introduced in GCC 4.4.  I'm not
> sure when the default C++ mode was changed to be >= -std=c++11.  It
> looks to me that the configure stuff should be making it pass
> `-std=c++11` if that's required, but I don't know how that test is
> coming out in your build.  I wonder if using `CXX=g++ -std=c++11`
> makes a difference with your version.