[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

I found this warning to be really useful but was confused that it is not always triggered.
I initially discovered -Wstring-plus-char (and fixed some issues in the projects I work on where sometimes developers didn't realized the "+" wasn't doing to do concatenation because it was applied on char and char*).
Then I also activated -Werror=string-plus-int to reduce the risk of developers doing mistakes in our codebase.

Turns out that because we had some buggy code not catch by this warning for this reason: the result of the concatenation was not out of bounds, so the warning wasn't triggered.

I understand that point of view, but IMHO this warning should be activated even though, like for -Wstring-plus-char: it's easy to fix the warning by having a nicer, more readable code, and IMO code raising this warning is likely to indicate an issue in the code.
Having developers accidentally concatenating string and integer can happen (even more if when not concatenating literals directly).
But I've found a bug in our codebase even more tricky: when concatenating enum and literals char* strings:
We had a code like this (using Qt framework):

  QString path = QStandardPaths::StandardLocation(QStandardPaths::DataLocation) + "/filename.ext";

The developer was trying to use QStandardPaths::standardLocations [1] static function (mind the lowercase s and the extra s at the end) which takes an enum of type QStandardPaths::StandardLocation. Similar name (so easy to do a typo)  but very different meanings.
So instead of getting a string object and concatenating it with some literal strings, path was set to a truncated version of "/filename.ext".
This kind of bugs is hard to catch during code review process (and wasn't in my case).

Long story, but I think it's interesting to illustrate the need to have this warning applied to more cases.

[1]: http://doc.qt.io/qt-5/qstandardpaths.html#standardLocations


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382