diff mbox series

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

Message ID CAB=4xhruhWimOF8v87bWnN-=Sw+EpV3UX0bugp6q2WW9xgu4hg@mail.gmail.com
State New
Headers
Series [users/roland/gold-charnn] gold: Use char16_t, char32_t instead of uint16_t, uint32_t as character types |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_check--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply

Commit Message

Roland McGrath Sept. 5, 2023, 7:35 p.m. UTC 
  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

Comments

Cary Coutant Sept. 6, 2023, 9:01 p.m. UTC | #1 
> 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
Roland McGrath Sept. 7, 2023, 12:22 a.m. UTC | #2 
Committed.

Thanks,
Roland
Vaseeharan Vinayagamoorthy Sept. 12, 2023, 12:37 p.m. UTC | #3 
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
Roland McGrath Sept. 20, 2023, 12:41 a.m. UTC | #4 
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.
Vaseeharan Vinayagamoorthy Oct. 19, 2023, 11:07 a.m. UTC | #5 
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
Roland McGrath Oct. 19, 2023, 7:30 p.m. UTC | #6 
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.
diff mbox series

Patch

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.