| Message ID | 20211217225825.499485-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Avoid narrowing in make_char_string_pack | expand |
On Fri, Dec 17, 2021 at 5:59 PM Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > This fixes > > gcc/cp/parser.c:4618:41: warning: narrowing conversion of '(char)(*(str + ((sizetype)i)))' from 'char' to 'unsigned char' [-Wnarrowing] > 4618 | unsigned char s[3] = { '\'', str[i], '\'' }; > | ~~~~~^ > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > Hi, I thought that GCC was built with -Wno-narrowing; has that changed since I last checked? I'd support a move to officially switch from disabling -Wnarrowing to enabling it instead if that's possible and hasn't been done yet. Also the '(char)(*(str + ((sizetype)i)))' looks like some implementation details leaking; is there a bug against -Wnarrowing (or the diagnostics system in general) open about that? > gcc/cp/ChangeLog: > > * parser.c (make_char_string_pack): Add a cast to const unsigned > char *. > --- > gcc/cp/parser.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 44eed7ea638..56232ab029f 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -4607,7 +4607,8 @@ make_char_string_pack (tree value) > { > tree charvec; > tree argpack = make_node (NONTYPE_ARGUMENT_PACK); > - const char *str = TREE_STRING_POINTER (value); > + const unsigned char *str > + = (const unsigned char *) TREE_STRING_POINTER (value); > int i, len = TREE_STRING_LENGTH (value) - 1; > tree argvec = make_tree_vec (1); > > > base-commit: d7ca2a79b82c6500ead6ab983d14c609e2124eee > -- > 2.33.1 >
On 12/17/21 17:58, Marek Polacek wrote: > This fixes > > gcc/cp/parser.c:4618:41: warning: narrowing conversion of '(char)(*(str + ((sizetype)i)))' from 'char' to 'unsigned char' [-Wnarrowing] > 4618 | unsigned char s[3] = { '\'', str[i], '\'' }; > | ~~~~~^ > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? Hmm, odd that STRING_CST and cpp_string differ in the use of unsigned char. The patch is OK> > gcc/cp/ChangeLog: > > * parser.c (make_char_string_pack): Add a cast to const unsigned > char *. > --- > gcc/cp/parser.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 44eed7ea638..56232ab029f 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -4607,7 +4607,8 @@ make_char_string_pack (tree value) > { > tree charvec; > tree argpack = make_node (NONTYPE_ARGUMENT_PACK); > - const char *str = TREE_STRING_POINTER (value); > + const unsigned char *str > + = (const unsigned char *) TREE_STRING_POINTER (value); > int i, len = TREE_STRING_LENGTH (value) - 1; > tree argvec = make_tree_vec (1); > > > base-commit: d7ca2a79b82c6500ead6ab983d14c609e2124eee
On Sat, Dec 18, 2021 at 07:43:55PM -0500, Eric Gallager wrote: > On Fri, Dec 17, 2021 at 5:59 PM Marek Polacek via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > This fixes > > > > gcc/cp/parser.c:4618:41: warning: narrowing conversion of '(char)(*(str + ((sizetype)i)))' from 'char' to 'unsigned char' [-Wnarrowing] > > 4618 | unsigned char s[3] = { '\'', str[i], '\'' }; > > | ~~~~~^ > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > Hi, I thought that GCC was built with -Wno-narrowing; has that changed > since I last checked? No, I noticed that warning when compiling parser.i manually. Here's a thread I found which led to -Wno-narrowing: <https://gcc.gnu.org/pipermail/gcc-patches/2011-October/326369.html>. > I'd support a move to officially switch from > disabling -Wnarrowing to enabling it instead if that's possible and > hasn't been done yet. I don't know if the issues above have been fixed. Maybe they have; it's been a while... > Also the '(char)(*(str + ((sizetype)i)))' looks > like some implementation details leaking; is there a bug against > -Wnarrowing (or the diagnostics system in general) open about that? Dunno. > > gcc/cp/ChangeLog: > > > > * parser.c (make_char_string_pack): Add a cast to const unsigned > > char *. > > --- > > gcc/cp/parser.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > > index 44eed7ea638..56232ab029f 100644 > > --- a/gcc/cp/parser.c > > +++ b/gcc/cp/parser.c > > @@ -4607,7 +4607,8 @@ make_char_string_pack (tree value) > > { > > tree charvec; > > tree argpack = make_node (NONTYPE_ARGUMENT_PACK); > > - const char *str = TREE_STRING_POINTER (value); > > + const unsigned char *str > > + = (const unsigned char *) TREE_STRING_POINTER (value); > > int i, len = TREE_STRING_LENGTH (value) - 1; > > tree argvec = make_tree_vec (1); > > > > > > base-commit: d7ca2a79b82c6500ead6ab983d14c609e2124eee > > -- > > 2.33.1 > > > Marek
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 44eed7ea638..56232ab029f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -4607,7 +4607,8 @@ make_char_string_pack (tree value) { tree charvec; tree argpack = make_node (NONTYPE_ARGUMENT_PACK); - const char *str = TREE_STRING_POINTER (value); + const unsigned char *str + = (const unsigned char *) TREE_STRING_POINTER (value); int i, len = TREE_STRING_LENGTH (value) - 1; tree argvec = make_tree_vec (1);
This fixes gcc/cp/parser.c:4618:41: warning: narrowing conversion of '(char)(*(str + ((sizetype)i)))' from 'char' to 'unsigned char' [-Wnarrowing] 4618 | unsigned char s[3] = { '\'', str[i], '\'' }; | ~~~~~^ Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? gcc/cp/ChangeLog: * parser.c (make_char_string_pack): Add a cast to const unsigned char *. --- gcc/cp/parser.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) base-commit: d7ca2a79b82c6500ead6ab983d14c609e2124eee