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

2018-12-12 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. 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 i

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

2018-12-13 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Firefox is good enough. Thanks for finding http://comments.gmane.org/gmane.comp.compilers.clang.scm/47203 -- this was from before we did reviews on phab, so I was able to find the review in m

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

2018-12-13 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL349054: Make -Wstring-plus-int warns even if when the result is not out of bounds (authored by sylvestre, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST

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

2018-12-13 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. 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). Repository: rL LLVM CHANGES

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

2018-12-13 Thread Adam Nemet via Phabricator via cfe-commits
anemet added a comment. This caused: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/56120/consoleFull#1420996271a1ca8a51-895e-46c6-af87-ce24fa4cd561 Please fix or revert. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://rev

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

2018-12-13 Thread Adam Nemet via Phabricator via cfe-commits
anemet added a comment. Actually this has been failing for 8 hours. So reverted in r349117. Also reverted your attempt to update the test. It wasn't updating the right test: r349118 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm

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

2018-12-13 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment. In D55382#1330668 , @anemet wrote: > Actually this has been failing for 8 hours. So reverted in r349117. Also > reverted your attempt to update the test. It wasn't updating the right test: > r349118 And I was just about to com

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

2018-12-14 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. 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 c

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

2018-12-18 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner reopened this revision. ArnaudBienner added a comment. This revision is now accepted and ready to land. Reopening since this broke some tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 __

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

2018-12-18 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 178624. ArnaudBienner added a comment. Herald added a subscriber: arphaman. Update tests broken by this change Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 Files: bindings/

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

2018-12-18 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. 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

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

2018-12-22 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision. thakis added a comment. Still looks fine to me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 ___ cfe-commits mailing list cfe-commits@lists.l

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

2019-01-03 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 180075. ArnaudBienner added a comment. Herald added a reviewer: serge-sans-paille. Make -Wstring-plus-int warns even if when the result is not out of bounds Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https

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

2019-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille resigned from this revision. serge-sans-paille added a comment. Just wanted to review the change to the Python script to ensure it remains compatible with Python2 and Python3, which is indeed the case; so LGTM on that aspect. Repository: rC Clang CHANGES SINCE LAST ACTION

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

2019-01-03 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC350335: Make -Wstring-plus-int warns even if when the result is not out of bounds (authored by abien, committed by ). Changed prior to commit: https://reviews.llvm.org/D55382?vs=180075&id=180093#toc Re

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

2019-01-03 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. OK, thanks Serge! :) For the record, I updated the patch one last time after it was accepted to remove my change to constant-expression-cxx1y.cpp since someone else did the same change in a separate commit meanwhile. Repository: rC Clang CHANGES SINCE LAST AC

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

2018-12-06 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D55382 Files: lib/Sema/SemaExpr.cpp test/SemaCXX/string-plus-int.cpp Index: test/SemaCXX/string-plus-int.cpp

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

2018-12-06 Thread 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 concatenatio

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

2018-12-06 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. 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. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/ne

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

2018-12-06 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. The usual process for adding / tweaking warnings is to build some large open-source project with the new / tweaked warning and note true and false positive rate. IIRC I did this on Chromium when I added the warning, and the out-of-bounds check prevented false positives w

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

2018-12-07 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. 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 poste