[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In https://reviews.llvm.org/D29839#674714, @cryptoad wrote: > So if I understand correctly, the consensus is to abandon this and rewrite it > to be part of the clang static analyzer? That's correct. https://reviews.llvm.org/D29839 ___

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-12 Thread Kostya Kortchinsky via Phabricator via cfe-commits
cryptoad added a comment. So if I understand correctly, the consensus is to abandon this and rewrite it to be part of the clang static analyzer? https://reviews.llvm.org/D29839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D29839#674517, @aaron.ballman wrote: > In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > > > Shouldn't this be a path sensitive check within t

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > Shouldn't this be a path sensitive check within the clang static analyzer > instead? So branches are properly handled and interprocedural analysis is > done. I agree; I think this check should be par

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-11 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29839#674315, @xazax.hun wrote: > In https://reviews.llvm.org/D29839#674307, @Prazek wrote: > > > In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > > > > > Shouldn't this be a path sensitive check within the clang static analyzer

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In https://reviews.llvm.org/D29839#674307, @Prazek wrote: > In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > > > Shouldn't this be a path sensitive check within the clang static analyzer > > instead? So branches are properly handled and interprocedural an

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-11 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote: > Shouldn't this be a path sensitive check within the clang static analyzer > instead? So branches are properly handled and interprocedural analysis is > done. Do you have some examples? I would argue, that e

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Shouldn't this be a path sensitive check within the clang static analyzer instead? So branches are properly handled and interprocedural analysis is done. https://reviews.llvm.org/D29839 ___ cfe-commits mailing list cfe-co

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-10 Thread Kostya Kortchinsky via Phabricator via cfe-commits
cryptoad updated this revision to Diff 88048. cryptoad added a comment. Missing line separators. https://reviews.llvm.org/D29839 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/IstreamOverflowCheck.cpp clang-tidy/misc/IstreamOverflowCheck.h clang-tidy/misc/MiscTidyModule.cpp doc

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-10 Thread Kostya Kortchinsky via Phabricator via cfe-commits
cryptoad updated this revision to Diff 88047. cryptoad added a comment. Addressing first batch of comments. https://reviews.llvm.org/D29839 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/IstreamOverflowCheck.cpp clang-tidy/misc/IstreamOverflowCheck.h clang-tidy/misc/MiscTidyModul

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-10 Thread Kostya Kortchinsky via Phabricator via cfe-commits
cryptoad marked 5 inline comments as done. cryptoad added inline comments. Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:78-80 +if (!Arg->isIntegerConstantExpr(WidthSize, Context)) { + llvm::errs() << "Couldn't get width() size.\n"; +} Prazek

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-10 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. Nice check! :) Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:59-61 + if (ConstType) { +ArraySize = ConstType->getSize(); + } same here Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:78-80 +if (!A