User Details
- User Since
- Oct 25 2018, 7:00 AM (117 w, 6 d)
Jun 28 2019
Jun 19 2019
Jan 6 2019
Jan 3 2019
OK, thanks Serge! :)
Make -Wstring-plus-int warns even if when the result is not out of bounds
Dec 18 2018
Fixed two tests broken by this change:
- test_diagnostics.py: AFAICT we are not testing all warnings here, but just that warnings are emitted correctly. The "+1" didn't seem to be useful, since the warning triggered was about the const char* being converted to an int (and this warning still applies)
- test/SemaCXX/constant-expression-cxx1y.cpp: is a false positive for -Wstring-plus-int so use the preferred, equivalent syntax
Reopening since this broke some tests.
Update tests broken by this change
Dec 14 2018
Really sorry about breaking the build :( Thanks for reverting it.
Actually, I think we could fix that test by removing the " +1": AFAICT this test is checking that warnings are correctly emitted from clang, but not testing all the warnings. So the conversion from const char* to int is enough: no need to have an extra +1 at the end.
I will update my patch to update the test accordingly.
Dec 13 2018
Thanks for the archeological work and finding the conversation related to the initial patch :)
Interesting that the last case you mentioned is exactly the bug I had in my project (though in your case this would have been intended).
Dec 12 2018
So I built Firefox with my patched version of clang and the only place it found a warning was in ffmpeg [1], a case you already reported in [2] when you implemented that patch, so no new positive AFAICT.
Do you also want to try on Chromium and see how it goes? Or is Firefox enough? (it's a pretty big C++ codebase, so I would assume that's enough)
Dec 7 2018
I found the commit: [1] which links to a related discussion [2] but there isn't much details. I wasn't sure if there was other discussions.
I will try to build Firefox with this change, and see how it goes (just need to find some time to do it).
I'll keep you posted.
Dec 6 2018
Nico: I've added you as reviewer since you originally implemented this warning (thanks for this BTW :) as said, it's really helpful), but feel free to delegate to someone else.
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.
Nov 19 2018
Thanks for the review! :)
Could someone please land the patch for me? I don't have commit access.
Nov 15 2018
@rsmith do you have any thoughts about this change?
Oct 29 2018
Hi Davide,