On 08/01/18 19:07, Martin Sebor wrote: > On 08/01/2018 05:20 AM, Bernd Edlinger wrote: >> On 07/30/18 17:49, Joseph Myers wrote: >>> On Mon, 30 Jul 2018, Bernd Edlinger wrote: >>> >>>> Hi, >>>> >>>> this is how I would like to handle the over length strings issue in the C >>>> FE. >>>> If the string constant is exactly the right length and ends in one explicit >>>> NUL character, shorten it by one character. >>> >>> I don't think shortening should be limited to that case. I think the case >>> where the constant is longer than that (and so gets an unconditional >>> pedwarn) should also have it shortened - any constant that doesn't fit in >>> the object being initialized should be shortened to fit, whether diagnosed >>> or not, we should define GENERIC / GIMPLE to disallow too-large string >>> constants in initializers, and should add an assertion somewhere in the >>> middle-end that no too-large string constants reach it. >>> >> >> Okay, there is an update following your suggestion. >> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >> Is it OK for trunk? > > The ChangeLog description says: > > * c-typeck.c (digest_init): Fix overlength strings. > > suggesting there is a bug but there is no test case. If there > is a bug in there that can be triggered by C code (valid or > otherwise), it would be good to have a test case and a bug > in Bugzilla. If there is no bug and this is just cleanup, > I would suggest to adjust the description. >
Yes, thanks for looking at this. This is an attempt to reduce the number of different encodings for otherwise identical strings in the middle-end. I could say "Shorten overlength strings." is that better? There are already numerous test cases with overlength strings. I verified that, because I have tested this patch on top of https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00050.html > Other than that, while making improvements here, I think it > would be helpful to also add more detail to the text of > the warning: > Sure, but it is important to do only one thing at a time. I see this as a preparation to fixing the remaining string_constant folding issues which are potential wrong-code issues. > 1) mention the type of the array being initialized in case > it's not obvious from the declaration (the array bound could > be a symbol, not a literal, or the type could be a typedef) > > 2) mention the number of elements in the initializer in case > it's a macro (such as __FILE__) whose definition isn't visible > in the diagnostic > > 3) mention that the excess elements are ignored (since it's > undefined in the standard, it will let users know what > happens in GCC). > > Here's a test case and a suggested warning: > > #define S __FILE__ "\000" > enum { N = sizeof __FILE__ }; > const char a[N] = S; > > warning: discarding 1 excess element from initializer-string for 'char[4]' > [-Wc++-compat] > #define S __FILE__ "\000" > ^~~~~~~~ > note: in expansion of macro ‘S’ > const char a[N] = S; > ^ > (Similarly for more than 1 excess element.) > Yes, definitely helpful, but not part of this patch. Probably one of your next patches, I would assume. Bernd.
