On Tue, Sep 23, 2014 at 03:52:21AM -0400, Eric Sunshine wrote:
> > That is my reading from the warning text, too, but I have to wonder:
> > wouldn't that mean they should be warning about pointer + pointer, not
> > pointer + int?
>
> 'pointer + pointer' is not legal C, is it? What would the result
> represent? The compiler correctly diagnoses that case as an error
> (without special command-line switches):
>
> error: invalid operands to binary expression
> ('char *' and 'char *')
> return "a" + "b";
> ~~~ ^ ~~~
You're correct that it's not legal. My point was more that "pointer +
int" is already clearly not string concatenation, because the operands
are not two strings.
I think the answer here (from the threads you linked below) is that
people expect it to not just concatenate, but also auto-convert integers
to strings. Yeesh.
> > Also, wouldn't the same advice apply to adding to _any_ char pointer,
> > not just a string literal?
>
> Not really. Indexing into a char array via pointer arithmetic is a
> perfectly reasonable and common idiom in C (indeed, git is peppered
> with it), so such a warning would be pure noise.
That is a good reason not to do the warning in these cases (and why I
hope that we will not have to deal with this further). But IMHO it is
good evidence that the warning is not well thought-out. It seems silly
that:
x = "foo" + 1;
is bad, but:
y = "foo";
x = y + 1;
is not[1]. Saying the first one is rare does not seem like a good
excuse; rather the existence of the second should tip you off that the
idiom is valid and not to be complained about.
Anyway, there is not much point in me complaining further about it here.
You are not the one who introduced it. :)
Thanks for digging in the history. It was interesting at least.
-Peff
[1] Actually, from reading the patch thread, I think the "1" needs to be
a non-constant integer here. But that just furthers the point: if
you have to neuter the warning to prevent a ton of false positives,
that is a good indication that you should not be warning. :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html