Message ID | orsff4srfy.fsf@lxoliva.fsfla.org |
---|---|
State | New |
Headers | show |
Series | [vxworks] make wint_t and wchar_t the same distinct type | expand |
On 2/17/23 23:04, Alexandre Oliva wrote: > > We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and > wchar_t mapped to the same underlying type, but this caused a glitch > in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to > wchar_t And fixing that isn't an option? Do the integer builtins work properly if we force them to use wchar_t instead of an integer type? > headers got included in the test that declared functions that > take wint_t parameters, and those conflicted with the builtin > declarations that had wint_t mapped to the underlying integral type. > > The problem is that, in C++, wchar_t is a distinct type. Having > wint_t be a typedef to wchar_t in the headers, but a typedef to > wchar_t's underlying integral type in builtins, makes for mismatches > between the declarations. > > This patch defines WINT_TYPE to "wchar_t" for vxworks, and adjusts the > fallout, namely: > > - since wchar_t may not have been defined yet when > c_common_nodes_and_builtins runs, use the node already reserved for > wchar_t for wint_t when WINT_TYPE is defined to wchar_t. > > - for the same reason, when WINT_TYPE is wchar_t and we're not > compiling C++ where wchar_t is a compiler built-in, define > __WINT_TYPE__ to WCHAR_TYPE rather than WINT_TYPE, because wchar_t > may not even be defined in the translation unit. > > - recognize and handle wchar_type_node when type_suffix is called for > wint_type_node. > > Regstrapped on x86_64-linux-gnu. > Tested on arm-vxworks7 (gcc-12) and arm-eabi (trunk). Ok to install? > > for gcc/ChangeLog > > * config/vx-common.h (WINT_TYPE): Alias to "wchar_t". > > for gcc/c-family/ChangeLog > > * c-common.cc (c_common_nodes_and_builtins): Take > wchar_type_node for wint_type_node when aliased. > (c_stddef_cpp_builtins): Define __WINT_TYPE__, when aliased to > wchar_t, to the underlying type rather than wchar_t in > non-C++. > * c-cppbuiltin.cc (type_suffix): Handle wchar_type_node. > --- > gcc/c-family/c-common.cc | 16 +++++++++++++--- > gcc/c-family/c-cppbuiltin.cc | 2 ++ > gcc/config/vx-common.h | 2 +- > 3 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc > index ae92cd5adaf5e..a92597c2f544f 100644 > --- a/gcc/c-family/c-common.cc > +++ b/gcc/c-family/c-common.cc > @@ -4576,8 +4576,11 @@ c_common_nodes_and_builtins (void) > char32_array_type_node > = build_array_type (char32_type_node, array_domain_type); > > - wint_type_node = > - TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE))); > + if (strcmp (WINT_TYPE, "wchar_t") == 0) > + wint_type_node = wchar_type_node; > + else > + wint_type_node = > + TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE))); > > intmax_type_node = > TREE_TYPE (identifier_global_value (get_identifier (INTMAX_TYPE))); > @@ -5359,7 +5362,14 @@ c_stddef_cpp_builtins(void) > builtin_define_with_value ("__SIZE_TYPE__", SIZE_TYPE, 0); > builtin_define_with_value ("__PTRDIFF_TYPE__", PTRDIFF_TYPE, 0); > builtin_define_with_value ("__WCHAR_TYPE__", MODIFIED_WCHAR_TYPE, 0); > - builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0); > + /* C++ has wchar_t as a builtin type, C doesn't, so if WINT_TYPE > + maps to wchar_t, define it to the underlying WCHAR_TYPE in C, and > + to wchar_t in C++, so the desired type equivalence holds. */ > + if (!c_dialect_cxx () > + && strcmp (WINT_TYPE, "wchar_t") == 0) > + builtin_define_with_value ("__WINT_TYPE__", WCHAR_TYPE, 0); > + else > + builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0); > builtin_define_with_value ("__INTMAX_TYPE__", INTMAX_TYPE, 0); > builtin_define_with_value ("__UINTMAX_TYPE__", UINTMAX_TYPE, 0); > if (flag_char8_t) > diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc > index b333f97fd3237..98f5aef2af95d 100644 > --- a/gcc/c-family/c-cppbuiltin.cc > +++ b/gcc/c-family/c-cppbuiltin.cc > @@ -1903,6 +1903,8 @@ type_suffix (tree type) > systems use it anyway. */ > || type == char_type_node) > is_long = 0; > + else if (type == wchar_type_node) > + return type_suffix (underlying_wchar_type_node); > else > gcc_unreachable (); > > diff --git a/gcc/config/vx-common.h b/gcc/config/vx-common.h > index 83580d0dec288..9733c90fe4c6f 100644 > --- a/gcc/config/vx-common.h > +++ b/gcc/config/vx-common.h > @@ -69,7 +69,7 @@ along with GCC; see the file COPYING3. If not see > #undef WINT_TYPE_SIZE > #define WINT_TYPE_SIZE WCHAR_TYPE_SIZE > #undef WINT_TYPE > -#define WINT_TYPE WCHAR_TYPE > +#define WINT_TYPE "wchar_t" > > /* ---------------------- Debug and unwind info formats ------------------ */ > >
Hello, Jason, On Feb 17, 2023, Jason Merrill <jason@redhat.com> wrote: > On 2/17/23 23:04, Alexandre Oliva wrote: >> >> We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and >> wchar_t mapped to the same underlying type, but this caused a glitch >> in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to >> wchar_t > And fixing that isn't an option? Erhm, why do you say "fixing"? That implies it's broken, but I don't see anything in the C++ standards, or in the relevant bits that it imports from the C standards, that rules out using wint_t = wchar_t in C++. wint_t is imported from the C standard as an integral type that meets certain requirements, and AFAICT in C++ wchar_t is an integral type that meets those requirements. Am I missing something? Now, could it be changed so that wint_t is wchar_t's underlying type rather than wchar_t? If the equivalence is a compliance error, we could file a bug report with WRS and request them to fix it, but modifying their system headers would require a copyright license they don't grant, so we avoid doing that. I imagine that breaking this equivalence would have ABI implications, and even break legitimate (though unportable) programs because of overload, specializations and whatnot, so there would have to be very strong reasons to support a request for such a change. > Do the integer builtins work properly if we force them to use wchar_t > instead of an integer type? I haven't observed any regressions, I don't see any builtin functions with wint in their signature that we even expand as builtins, and my imagination is failing me on why an integral type such as wchar_t would fail for wint_t, where other integral types, including wchar_t's underlying type, would work. Did you have any specific risks in mind about what could go wrong?
On 2/22/23 10:23, Alexandre Oliva wrote: > Hello, Jason, > > On Feb 17, 2023, Jason Merrill <jason@redhat.com> wrote: > >> On 2/17/23 23:04, Alexandre Oliva wrote: >>> >>> We used to define WINT_TYPE to WCHAR_TYPE, so that both wint_t and >>> wchar_t mapped to the same underlying type, but this caused a glitch >>> in Wstringop-overflow-6.C: on vxworks, wint_t is typedef'ed to >>> wchar_t > >> And fixing that isn't an option? > > Erhm, why do you say "fixing"? That implies it's broken, but I don't > see anything in the C++ standards, or in the relevant bits that it > imports from the C standards, that rules out using wint_t = wchar_t in > C++. wint_t is imported from the C standard as an integral type that > meets certain requirements, and AFAICT in C++ wchar_t is an integral > type that meets those requirements. Am I missing something? > > > Now, could it be changed so that wint_t is wchar_t's underlying type > rather than wchar_t? If the equivalence is a compliance error, we could > file a bug report with WRS and request them to fix it, but modifying > their system headers would require a copyright license they don't grant, > so we avoid doing that. I imagine that breaking this equivalence would > have ABI implications, and even break legitimate (though unportable) > programs because of overload, specializations and whatnot, so there > would have to be very strong reasons to support a request for such a > change. > >> Do the integer builtins work properly if we force them to use wchar_t >> instead of an integer type? > > I haven't observed any regressions, I don't see any builtin functions > with wint in their signature that we even expand as builtins, and my > imagination is failing me on why an integral type such as wchar_t would > fail for wint_t, where other integral types, including wchar_t's > underlying type, would work. Did you have any specific risks in mind > about what could go wrong? Sorry, I think I was mostly confusing myself about how distinct wchar_t really is. The patch is OK. Jason
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc index ae92cd5adaf5e..a92597c2f544f 100644 --- a/gcc/c-family/c-common.cc +++ b/gcc/c-family/c-common.cc @@ -4576,8 +4576,11 @@ c_common_nodes_and_builtins (void) char32_array_type_node = build_array_type (char32_type_node, array_domain_type); - wint_type_node = - TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE))); + if (strcmp (WINT_TYPE, "wchar_t") == 0) + wint_type_node = wchar_type_node; + else + wint_type_node = + TREE_TYPE (identifier_global_value (get_identifier (WINT_TYPE))); intmax_type_node = TREE_TYPE (identifier_global_value (get_identifier (INTMAX_TYPE))); @@ -5359,7 +5362,14 @@ c_stddef_cpp_builtins(void) builtin_define_with_value ("__SIZE_TYPE__", SIZE_TYPE, 0); builtin_define_with_value ("__PTRDIFF_TYPE__", PTRDIFF_TYPE, 0); builtin_define_with_value ("__WCHAR_TYPE__", MODIFIED_WCHAR_TYPE, 0); - builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0); + /* C++ has wchar_t as a builtin type, C doesn't, so if WINT_TYPE + maps to wchar_t, define it to the underlying WCHAR_TYPE in C, and + to wchar_t in C++, so the desired type equivalence holds. */ + if (!c_dialect_cxx () + && strcmp (WINT_TYPE, "wchar_t") == 0) + builtin_define_with_value ("__WINT_TYPE__", WCHAR_TYPE, 0); + else + builtin_define_with_value ("__WINT_TYPE__", WINT_TYPE, 0); builtin_define_with_value ("__INTMAX_TYPE__", INTMAX_TYPE, 0); builtin_define_with_value ("__UINTMAX_TYPE__", UINTMAX_TYPE, 0); if (flag_char8_t) diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc index b333f97fd3237..98f5aef2af95d 100644 --- a/gcc/c-family/c-cppbuiltin.cc +++ b/gcc/c-family/c-cppbuiltin.cc @@ -1903,6 +1903,8 @@ type_suffix (tree type) systems use it anyway. */ || type == char_type_node) is_long = 0; + else if (type == wchar_type_node) + return type_suffix (underlying_wchar_type_node); else gcc_unreachable (); diff --git a/gcc/config/vx-common.h b/gcc/config/vx-common.h index 83580d0dec288..9733c90fe4c6f 100644 --- a/gcc/config/vx-common.h +++ b/gcc/config/vx-common.h @@ -69,7 +69,7 @@ along with GCC; see the file COPYING3. If not see #undef WINT_TYPE_SIZE #define WINT_TYPE_SIZE WCHAR_TYPE_SIZE #undef WINT_TYPE -#define WINT_TYPE WCHAR_TYPE +#define WINT_TYPE "wchar_t" /* ---------------------- Debug and unwind info formats ------------------ */