[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:855 + + CurrentSFC = Node->getStackFrame(); + NoQ wrote: > Mmm, wait a sec. This way the loop condition is trivially

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:170-171 super(Store, self).__init__() -self.clusters = [StoreCluster(c) for c in json_s] +self.clusters = collections.Or

[PATCH] D63362: [analyzer] Fix JSON dumps for store clusters.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Thanks you! Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:215 + << "{ \"cluster\": \"" << I.getKey() << "\", \"pointer\": \"" + << ((const v

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

This revision was automatically updated to reflect the committed changes. Closed by commit rL363491: [analyzer] ReturnVisitor: Bypass everything to see inlined calls (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to comm

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

Charusso marked an inline comment as done. Charusso added a comment. In D62926#1545163 , @hintonda wrote: > This test fails to compile on Windows 64 bit builds. Please see > http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/77 Thanks you!

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

Charusso marked 3 inline comments as done. Charusso added a comment. @NoQ, thanks for the review! Now everything is working by rL363515 . Comment at: cfe/trunk/test/Analysis/inlining/placement-new-fp-suppression.cpp:16 + +typedef unsigned lon

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

Charusso requested changes to this revision. Charusso added a comment. This revision now requires changes to proceed. Hey! It is a cool patch as the smallest the scope the better to understand what is going on. I like the direction, but we saw with @NoQ, we have to think about "In which range do

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

Charusso added a comment. In D62883#1545341 , @Szelethus wrote: > In D62883#1545324 , @Charusso wrote: > > > As @NoQ pointed out we have some problem with that function. We are > > tracking *values* without using t

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

Charusso accepted this revision. Charusso added a comment. My opinion based on the Summary, which is the kind of the opposite what we have. No invalidation happens, so no swap happens, so it is kind of misleading like writing out "Returning something" where we cannot be sure what we return ("re

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. - Now we apply the `has_report` on the proper `ProgramPoint`. -

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/test/Analysis/dump_egraph.c:3 // RUN: cat %t.dot | FileCheck %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot -trim-egraph %s // REQUIRES: asserts ---

[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Now we also print out the filename with its path. Repository:

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

Charusso added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:287 .replace('\\}', '}') \ +.replace('', '\\') \ .replace('\\<

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

Charusso updated this revision to Diff 205208. Charusso edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63436/new/ https://reviews.llvm.org/D63436 Files: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/test/Analysis/dump_egraph.c Index: clang/t

[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

Charusso updated this revision to Diff 205209. Charusso added a comment. - Added a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63438/new/ https://reviews.llvm.org/D63438 Files: clang/lib/Analysis/ProgramPoint.cpp clang/test/Analysis/dump_egraph.c Index: clang/test/An

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. - Repository: rC Clang https://reviews.llvm.org/D63462 Fi

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

Charusso marked 2 inline comments as done. Charusso added a comment. Thanks for the review! Comment at: clang/test/Analysis/dump_egraph.c:3 // RUN: cat %t.dot | FileCheck %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot -trim-egraph %s // RE

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added a comment. Thanks for the main development! Could I start to reduce the size with my SVG-magic? What do you think about make it more SVG-based on the styling side? Comment at: clang/util

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

Charusso added a comment. In D63462#1549144 , @NoQ wrote: > Hmm, this doesn't seem to solve my problem in D62761 > . Let me write some actual test case so that > you knew it's fixed when it's fixed. Well, I have did the

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

Charusso added a comment. In D63462#1549225 , @NoQ wrote: > I mean, i'm removing backslashes but you're adding more backslashes. > Therefore i think we're talking about different issues. You *have to* remove backslashes, because sometimes we have ``

[PATCH] D63519: [analyzer] exploded-graph-rewriter: Fix escaping and unescaping of StringRegions.

Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:222 +Indent(Out, Space, IsDot) << "{ " << CI.getKey()

[PATCH] D64611: [analyzer] exploded-graph-rewriter: Improve source location dumps.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Much better! Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64611/new/ https://reviews.llvm.org/D64611 __

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Charusso accepted this revision. Charusso added a comment. The `impure-warning` sounds like some alpha, not-well-defined warning, other than that I like the movement to rethink existing checkers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Integer Set Library using retain-count based allocation which i

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 209654. Charusso marked 2 inline comments as done. Charusso added a comment. - Fix. - Move the logic to `free()` for better matching. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/Stati

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added a comment. In D64680#1584076 , @NoQ wrote: > Change of plans: let's suppress the warning when our `free()` is done within > the function that has `__isl_take` in its definition. So, like, ascend the > chain of location contexts and check y

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added a comment. Here is an example of the mentioned use-after-free by pointer-escaping as an argument: https://llvm.org/reports/scan-build/report-DeclBase.cpp-getFromVoidPointer-0-1.html#EndPath CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 209849. Charusso added a comment. - Remove unnecessary `DoNothing` kind. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/re

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso marked 4 inline comments as done. Charusso added a comment. In D64680#1584315 , @NoQ wrote: > P.S. I think you should attach the report to Phabricator directly, as the > link will expire as soon as these reports get regenerated. Luckily the sta

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 210347. Charusso marked 9 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552 + FunctionStr = Lexer::getSourceText( + CharSourceRange::getTokenRange( + {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}), + C.getSourc

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 210458. Charusso marked 4 inline comments as done. Charusso added a comment. - More fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552 + FunctionStr = Lexer::getSourceText( + CharSourceRange::getTokenRange( + {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}), + C.getSourc

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added a comment. In D64680#1590619 , @NoQ wrote: > Great, thanks! Thanks for the review! I like that new name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 ___

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

This revision was automatically updated to reflect the committed changes. Closed by commit rL366391: [analyzer] MallocChecker: Prevent Integer Set Library false positives (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. We do not support evaluating bitwise operations, so that when w

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

Charusso accepted this revision. Charusso added a comment. When I started programming I read that the programming is British English, but LLVM project has more 'behavior' than 'behaviour', so as being democratic it should be fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65180/new

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

Charusso added a comment. In D65250#1601187 , @NoQ wrote: > In D65250#1600776 , @grandinj wrote: > > > There is no need to wrap SVG in HTML if you want to display it in a > > web-browser > > > I guess it's worth it

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso updated this revision to Diff 211956. Charusso edited the summary of this revision. Charusso added a comment. - Restrict the generic contradiction-based range evaluation to only affect that left-hand side operands which constraint range are concrete zero. CHANGES SINCE LAST ACTION ht

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso marked 3 inline comments as done. Charusso added a comment. In D65239#1599889 , @NoQ wrote: > Aha, great, the overall structure of the code is correct! Thanks! Now it is more formal. The only problem it does not change anything on LLVM reports,

[PATCH] D65344: [analyzer] exploded-graph-rewriter: NFC: Replace explorers with trimmers.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. In my mind an explorer would trim me a graph so I like the new abstraction to differentiate the two. Thanks! Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I like the HTML output, thanks! Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:846 +print(' $ dot -Tsvg input.dot -o output.svg') +

[PATCH] D65345: [analyzer] exploded-graph-rewriter: Implement manual graph trimming.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. It is a great idea, thanks! Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:918 +# TargetedTrimmer keeps paths that lead to specific nodes and discards all +#

[PATCH] D65427: [analyzer] exploded-graph-rewriter: Implement Store pointers.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Thanks! Is it working in dark-mode as expected? Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:647 else: +self._dump(' (%s)' % st.ptr)

[PATCH] D65427: [analyzer] exploded-graph-rewriter: Implement Store pointers.

Charusso accepted this revision. Charusso added a comment. Oh, of course. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65427/new/ https://reviews.llvm.org/D65427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso updated this revision to Diff 212440. Charusso marked 6 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65239/new/ https://reviews.llvm.org/D65239 Files: clang/include/clang/AST/Expr.h clang/include/clang/StaticAnaly

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:496 + // as a bitwise operation result could be null. + if (RS.getConcreteValue() && RS.getConcreteValue()->getExtValue() == 0) +return State; NoQ wrote: >

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, zzheng, szepet, baloghadamsoftware, xazax.hun. With this addition we can distinguish between `StackSpa

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:486 const StackFrameContext *getStackFrame(AnalysisDeclContext *Ctx, - LocationContext const *Paren

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso updated this revision to Diff 212895. Charusso marked an inline comment as done. Charusso edited the summary of this revision. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65587/new/ https://reviews.llvm.org/D65587 Files: clang/include/clang

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso added a subscriber: george.karpenkov. Charusso added inline comments. Comment at: clang/test/Analysis/loop-unrolling.cpp:349-353 #ifdef DFS -clang_analyzer_numTimesReached(); // expected-warning {{10}} +clang_analyzer_numTimesReached(); // expected-warning {{16}

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso added a comment. Thanks for the review and for the idea! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65587/new/ https://reviews.llvm.org/D65587 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

This revision was automatically updated to reflect the committed changes. Closed by commit rL367608: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its… (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso added a comment. It also reverts https://reviews.llvm.org/rL362632 which is not a fix, but a problem instead. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 ___ cfe

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a comment. It also reverts https://reviews.llvm.

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso updated this revision to Diff 213610. Charusso marked 4 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/lib

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso added a comment. Here is an example of the new `MemberExpr::getBase()` based report: F9736772: report-Driver.cpp-operator()-6-1.html Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420 + if (!IsAssuming) { +P

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso updated this revision to Diff 213763. Charusso marked 2 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/lib

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso added a comment. Thanks for the review! It is much better: F9740817: report-Driver.cpp-operator()-6-2.html Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420 + if (!IsAssuming) { +PathDiagnosticLocation Loc(

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Thanks to Kristóf Umann for the great idea! Repository: rC

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso marked 4 inline comments as done. Charusso added a comment. This is a little-bit WIP as the symbol conjuring is very naive. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.ge

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added a comment. In D64374#1594617 , @Szelethus wrote: > I guess `getAs` and `castAs` methods could join the party too. I'm evaluating > plenty of results on LLVM, and those seem to be in the same problem category. In D64374#1618694

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } NoQ wrote: > Charusso wrote: > > That is a

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 213952. Charusso marked 5 inline comments as done. Charusso added a comment. - Remove symbol-conjuring. - Add a forgotten test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/Static

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } NoQ wrote: > Charusso wrote: > > NoQ wrote

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214037. Charusso marked 9 inline comments as done. Charusso added a comment. - Make it usable with references. - Test references. - Better messages on simple `cast<>`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.or

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214038. Charusso added a comment. - Fix a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp clang/test/Analysis/cast-value.cpp Index

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51 + + const CallDescriptionMap SugarCastCDM = { + {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs}, NoQ wrote: > I'd rather have only one map from c

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214041. Charusso added a comment. - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a class. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checke

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214186. Charusso added a comment. - The call as `getAsCXXRecordDecl()` sometimes run into an assertion failure, so we need to avoid it: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:333: ExprEngine::createTemporaryRegionIfNeeded(): Assertion `!InitValW

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214234. Charusso marked 7 inline comments as done. Charusso added a comment. - Added a test case for casting *to* a reference. - Better naming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; + NoQ wrote: > Please explain "Checking" and "Sugar". Checking what? Sugar

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214277. Charusso marked 6 inline comments as done. Charusso added a comment. - Better naming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; + NoQ wrote: > Charusso wrote: > > NoQ wrote: > > > Please explain "Checki

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added a comment. Thanks you for the review! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

This revision was automatically updated to reflect the committed changes. Closed by commit rL368382: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

This revision was automatically updated to reflect the committed changes. Closed by commit rL368383: [analyzer] CastValueChecker: Model castAs(), getAs() (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:

[PATCH] D65182: [analyzer] Add fix-it hint support.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I really like that patch, as no human would rewrite 600 `dyn_cast` and `getAs` to `cast` and `castAs`. Thanks you! Comment at: clang/include/clang/StaticAnalyzer/Checker

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This patch introduces a new option: `-analyzer-disable-warning`

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso updated this revision to Diff 214490. Charusso added a comment. - Remove one misleading 'no-warning' comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang/include/clang/Driv

[PATCH] D65182: [analyzer] Add fix-it hint support.

Charusso accepted this revision. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:103 void enableWerror() { ShouldEmitAsError = true; } + void enableFixitsAsRemarks() { FixitsAsRemarks = true; } NoQ wrote: >

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added inline comments. Comment at: clang/include/clang/Driver/CC1Options.td:127-128 +def analyzer_disable_warning : Separate<["-"], "analyzer-disable-warning">, + HelpText<"Choose analyzer checkers of the warnings to disable">; +def analyzer_disable_warning_EQ : Joine

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso updated this revision to Diff 214498. Charusso marked 5 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang/include/clang/

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso updated this revision to Diff 214500. Charusso added a comment. - Fix a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang/include/clang/Driver/CC1Options.td clang/incl

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:192-197 + /// Pair of checker name and enable/disable to do analysis. + std::vector> CheckerAnalysisVector; + + /// Vector of checker names to do not emit warnings. + std::ve

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624320 , @Szelethus wrote: > I think it would make a lot more sense to create a separate (and hidden!) > coreModeling package that would do all the modeling, and regard core as a > highly recommended, but not mandator

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624432 , @Szelethus wrote: > In D66042#1624365 , @Charusso wrote: > > > In D66042#1624320 , @Szelethus > > wrote: > > > > > I think it w

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624468 , @Szelethus wrote: > In D66042#1624462 , @Charusso wrote: > > > My solution preserve the checkers as they are and yours definitely would > > rewrite them. Checker writin

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624475 , @Szelethus wrote: > In D66042#1624469 , @Charusso wrote: > > > In a long-term rewriting the Analyzer from scratch would be ideal. There is > > no problem with this patc

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624684 , @NoQ wrote: > My idea here was that this new feature isn't going to be user-facing. We > aren't promising to support all combinations of > enabled-disabled-silenced-dependent-registercheckerhacks, but instead

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1625971 , @NoQ wrote: > In D66042#1625000 , @Szelethus wrote: > > > Your patch title, and the things things that you said make me believe that > > there are only a handful of cor

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1626122 , @NoQ wrote: > > use it locally > > What do you mean here? If you want to use the patch for evaluating your > results, you might as well untick the checker in the scan-build's index.html > interface. The point

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1625971 , @NoQ wrote: > In D66042#1625000 , @Szelethus wrote: > > > if we add this flag, people responsible for developing interafaces for the > > analyzer might end up using it.

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1626460 , @NoQ wrote: > I'd like to hear @Szelethus's opinion one more time on this patch. I'm > perfectly fine with abandoning the idea and going for in-checker > suppressions, simply because there's at least one stro

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. Any analyzer config flag is equally accessible to anyone as the driver flags as they are both flags. The only difference is the config flags are more code to implement, and a lot more difficult to use. @NoQ, why the hell would we pick another type of flag which makes z

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso updated this revision to Diff 215103. Charusso marked 4 inline comments as done. Charusso retitled this revision from "[analyzer] Analysis: "Disable" core checkers" to "[analyzer] Analysis: Silence checkers". Charusso edited the summary of this revision. Charusso set the repository for th

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso added a comment. I realized that it is meaningless what separator we use to list the checkers, so I have picked `;`. The `,` is limited to the `analyzer-config` list. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:192-197 + /// Pair of checker

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso updated this revision to Diff 215159. Charusso marked 4 inline comments as done. Charusso added a comment. - Rainbow Butterfly Unicorn Kitty CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 F

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimenta

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

<    1   2   3   4   5   6   7   >