Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-21 Thread Devin Coughlin via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL276365: [analyzer] Add checker modeling potential C++ self-assignment (authored by dcoughlin). Changed prior to commit: https://reviews.llvm.org/D19311?vs=64864&id=64996#toc Repository: rL LLVM http

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-21 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware removed rL LLVM as the repository for this revision. baloghadamsoftware updated this revision to Diff 64864. baloghadamsoftware added a comment. Bug path string fixed. https://reviews.llvm.org/D19311 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td include/clang/S

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-21 Thread Gábor Horváth via cfe-commits
xazax.hun added inline comments. Comment at: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1738 @@ +1737,3 @@ + + const auto Msg = "Assuming " + Met->getParamDecl(0)->getName() + + ((Param == This) ? " == " : " != ") + "*this"; getNa

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-20 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware added a comment. Thx, I checked the output, but I do not understand why a simple string concatenation fails in your test environment. It works on our build server (Linux) with the latest master trunk. Repository: rL LLVM https://reviews.llvm.org/D19311

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-20 Thread Devin Coughlin via cfe-commits
dcoughlin added a comment. Here is the failing test output in case the bot log goes away: - TEST 'Clang :: Analysis/self-assign.cpp' FAILED Script: --- /home/bb/cmake-clang-x86_64-linux/build/./bin/clang -cc1 -internal-isystem /home/bb/cmake-clang-x86_64-linux/build/bi

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-19 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware added a comment. Do I use an non-portable way to concatenate strings? "Assuming rhs == *this" becomes "0*this" for some strange reason. I tested it again with the latest master branch and all tests are passing like earlier. Repository: rL LLVM https://reviews.llvm.org/D19

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-18 Thread Devin Coughlin via cfe-commits
dcoughlin reopened this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Re-opening. Reverted in r275880. It was causing a failure on the bots: http://bb.pgr.jp/builders/cmake-clang-x86_64-linux/builds/51780 Repository: rL LLVM https://reviews.llvm.org/D1

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-18 Thread Devin Coughlin via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL275820: [analyzer] Add checker modeling potential C++ self-assignment (authored by dcoughlin). Changed prior to commit: https://reviews.llvm.org/D19311?vs=64135&id=64346#toc Repository: rL LLVM http

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-15 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware marked an inline comment as done. baloghadamsoftware added a comment. https://reviews.llvm.org/D19311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-15 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware updated this revision to Diff 64135. baloghadamsoftware added a comment. Test updated. https://reviews.llvm.org/D19311 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h lib/StaticAnalyzer/Checkers

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-14 Thread Devin Coughlin via cfe-commits
dcoughlin added a comment. Other than adding expected notes for the path notes to the test, this looks good to me. Thanks Ádám! Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1698 @@ +1697,3 @@ +PathDiagnosticPiece * +CXXSelfAssignmentBRVisitor::VisitNode(const Exp

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-08 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware added a comment. I added tests for move assignment operators, but I could not find out any other simple test case than memory leak. However memory leaks are currently only detected by Unix.malloc for malloc. So I tried to replace strdup with malloc, strlen and strcpy, but eve

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-08 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware added inline comments. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:452 @@ -444,5 +451,3 @@ // inlining when reanalyzing an already inlined function. - if (Visited.count(D)) { -assert(isa(D) && - "We are only reanalyzing ObjCMet

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-08 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware updated this revision to Diff 63207. baloghadamsoftware added a comment. Debug line removed. http://reviews.llvm.org/D19311 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h lib/StaticAnalyzer/Che

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-08 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware marked 3 inline comments as done. baloghadamsoftware added a comment. http://reviews.llvm.org/D19311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-07-08 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware updated this revision to Diff 63204. baloghadamsoftware added a comment. Issues fixed. http://reviews.llvm.org/D19311 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h lib/StaticAnalyzer/Checkers/

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-05-17 Thread Devin Coughlin via cfe-commits
dcoughlin added a comment. Thanks for the patch! This looks like it will catch some nasty bugs. My comments (inline) are mostly nits. But two are more substantial: (1) you should add a path note where the self-assignment assumption is made explaining the assumption to the user and (2) the poten

Re: [PATCH] D19311: [analyzer] Self Assignment Checker

2016-04-26 Thread Balogh , Ádám via cfe-commits
baloghadamsoftware updated this revision to Diff 54968. baloghadamsoftware added a comment. Initial comments added to the checker and tests are converted from (DOS) to (Unix) format. http://reviews.llvm.org/D19311 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Check