[PATCH] D65196: [CFG] Introduce CFGElementRef, a wrapper that knows it's position in a CFGBlock

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Oof. Someone had to do it. Comment at: clang/include/clang/Analysis/CFG.h:617 + template + class ElementRef { + Yeah, kinda shocking that you needed to go that

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

NoQ updated this revision to Diff 211639. NoQ added a comment. Make it a soft failure when graphviz is not installed. Display a friendly warning. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65250/new/ https://reviews.llvm.org/D65250 Files: clang/test/Analysis/exploded-graph-rewrit

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

NoQ updated this revision to Diff 211640. NoQ added a comment. Polish wording a bit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65250/new/ https://reviews.llvm.org/D65250 Files: clang/test/Analysis/exploded-graph-rewriter/lit.local.cfg clang/utils/analyzer/exploded-graph-rewrite

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

NoQ added a comment. In D65250#1600776 , @grandinj wrote: > There is no need to wrap SVG in HTML if you want to display it in a > web-browser, you can just open the web-browser explicitly, they already > support opening SVG, use something like > > webb

[PATCH] D65290: [analyzer][NFC] Prove that we only track the evaluated part of the condition

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Just as planned! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65290/new/ https://reviews.llvm.org/D65290 _

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

NoQ added a comment. I'm sure i've seen something with a do-while(0) loop. Let's also add the following: #define assert(x) do {\ if (!x) \ __assert_fail (#expr, __FILE__, __LINE__, __func__) \ } w

[PATCH] D65106: [OPENMP]Add support for analysis of reduction variables.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Herald added a reviewer: jdoerfert. Great, thanks! That'll be fun to model in the static analyzer :/ Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65106/new/ https:

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

NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Explorers aren't the right abstraction. For the purposes of displaying

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

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. NoQ added a parent revision: D65344: [a

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

NoQ updated this revision to Diff 211992. NoQ added a comment. Fxd. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65344/new/ https://reviews.llvm.org/D65344 Files: clang/test/Analysis/exploded-graph-rewriter/explorers.dot clang/test/Analysis/exploded-graph-rewriter/trimmers.dot c

[PATCH] D65349: [analyzer] Be more careful with destructors of non-regions.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. It turns out that we crash all over th

[PATCH] D50256: [Analyzer] Basic support for multiplication and division in the constraint manager (for == and != only)

NoQ added inline comments. Comment at: test/Analysis/multiplicative-folding.c:140-142 +clang_analyzer_eval(n == -1); //expected-warning{{FALSE}} +clang_analyzer_eval(n == 0); //expected-warning{{TRUE}} +clang_analyzer_eval(n == 1); //expected-warning{{FALSE}}

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, JDevlieghere, szepet. Herald added a project: clang. This is inspired by http

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:157 ClusterBindings::Factory *CBFactory; + bool IsMainAnalysis; We could have put this flag into `RegionStoreManager` instead - after

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

NoQ updated this revision to Diff 212035. NoQ added a comment. Actually add the logic for C++. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65361/new/ https://reviews.llvm.org/D65361 Files: clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/test/Analysis/main.c clang/test/Analy

[PATCH] D65349: [analyzer] Be more careful with destructors of non-regions.

NoQ added a comment. In D65349#1604363 , @baloghadamsoftware wrote: > Is there any real-world use-case for casting concrete integers to class > instances? How did you find this crashing case? I think in original code this value was produced by doing po

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

NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Those are useful for understanding contents of `LazyCompoundVal`s. F9

[PATCH] D65378: [analyzer][NFC] Refactoring BugReporter.cpp P1.: Store interesting symbols/regions in a simple set

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I dunno why rC162028 was doing this. That's some advanced archeology down there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

NoQ updated this revision to Diff 212257. NoQ added a comment. Hmm, on second thought... Make the pointers a bit darker in dark mode. F9692102: Screen Shot 2019-07-29 at 5.31.14 PM.png CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65427/new/ https:/

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

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:647 else: +self._dump(' (%s)' % st.ptr) if prev_st is not None: Charusso wrote: > Extra space at the sta

[PATCH] D65379: [analyzer][NFC] Refactoring BugReporter.cpp P2.: Clean up the construction of bug paths and finding a valid report

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. If only somebody could explain to me why do even need trimming in the first place :/ I was only keeping it around for easier exploded graph dumps reading, but these days we can trivially replace it

[PATCH] D65381: [analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer -> PathDiagnosticPieceRef

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. > find clang/ -type f -exec sed -i > 's/std::shared_ptr/PathDiagnosticPieceRef/g' {} \; > git diff -U3 --no-color HEAD^ | clang-format-diff -p1 -i All you need to know about the state of C++ ref

[PATCH] D65382: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it const

NoQ added a comment. It's counter-idiomatic to pass manager objects (`ProgramStateManager`, `SValBuilder`, etc.) as const-references because it tells you pretty much nothing about what you can or cannot do with them. You don't really ever semantically mutate them in the first place. But when i

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

NoQ 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; Instead of "we kno

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

NoQ added a comment. In D65182#1604280 , @baloghadamsoftware wrote: > Hmm, I was thinking on the same for some time but I wonder how many checkers > could find the correct fixits? Maybe the removal fixits of double frees or > double file closes, but I a

[PATCH] D65382: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it const

NoQ added a comment. In D65382#1605769 , @baloghadamsoftware wrote: > Most managers are stateful because they also store the elements they manage > (e.g. `ProgramStateManager` stores states, `SValBuilder` owns other managers > such as `SymbolManager` th

[PATCH] D62525: [Analyzer] Add new visitor to the iterator checkers

NoQ added a comment. In D62525#1607813 , @Szelethus wrote: > In D62525#1523026 , > @baloghadamsoftware wrote: > > > The problem is that the transition is added in the top level function but > > iterator position a

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Ah, omission of the century :/ Thanks, this looks immediately good! Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:305 + // The 'NodeBuilderContext::blockCount(

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

NoQ added a comment. Fantastic! Let's open the wording bikeshed season? I suspect that a simple "(The) Value -> Condition value" change would have worked better. Another variant: "Value ..., which participates in a condition later". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Sry, should have approved this ages ago >.< Comment at: clang/test/Analysis/uninit-vals.c:181 void testUseHalfPoint() { - struct Point p = getHalfPoint(); // expected-note{{Calli

[PATCH] D65487: [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I thereby confirm that i've no idea what was this code about. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65487/new/ https://reviews.llvm.org/D65487

[PATCH] D65650: [Analyzer] Iterator Checkers - Fix for Crash on Iterator Differences

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Fair enough! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65650/new/ https://reviews.llvm.org/D65650 ___ cfe-commits

[PATCH] D62445: [test] Fix plugin tests

NoQ added a comment. Ugh, there seems to be one more forgotten buildbot with plugins problems: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6406/consoleText FAIL: Clang :: Analysis/checker-plugins.c (467 of 14955) TEST 'Clang :: Analysis/checker-plugins.c

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

NoQ added a comment. Aha, aha, looks reasonable. If we're describing a variable, we should only highlight that variable. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420 + if (!IsAssuming) { +PathDiagnosticLocation Loc(BExpr->getLHS(), BRC.getSourceM

[PATCH] D65382: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it const

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Do it! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65382/new/ https://reviews.llvm.org/D65382 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D65484: [analyzer][NFC] Refactoring BugReporter.cpp P5.: Compact mile long function invocations into objects

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. *bows down to those who bestow documentation upon this code* Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:111 +/// getters and some well placed asserts for extra secur

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

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420 + if (!IsAssuming) { +PathDiagnosticLocation Loc(BExpr->getLHS(), BRC.getSourceManager(), LCtx); return std::make_shared(Loc, Message); Charusso wrote: > N

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yay! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 ___ cfe-commits mailing list cfe-commits@lists.l

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

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

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

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

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

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

[PATCH] D65461: [OPENMP]Add support for analysis of linear variables and step.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thx again! Comment at: lib/AST/OpenMPClause.cpp:473 + // (Step and CalcStep), list of used expression + step. + void *Mem = C.Allocate(totalSizeToAlloc(5 * NumVars + 2 + NumVars

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

NoQ added a comment. Nice!! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:225-226 // FIXME: constexpr initialization isn't supported by MSVC2013. - static const char *const GenericTrueMessage; - static const char *const GenericFalse

[PATCH] D65725: [analyzer] Mention whether an event is about a condition in a bug report part 2

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Accept as long as we agree on the wording as part of D65575 . Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:465 void as

[PATCH] D65723: [analyzer][NFC] Add different interestingness kinds

NoQ accepted this revision. NoQ added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:92-101 /// Default tracking kind -- specifies that as much information should be //

[PATCH] D65578: [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces

NoQ accepted this revision. NoQ added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:108 SmallVector Ranges; + const SourceRange ErrorNodeRange; ExtraTextList ExtraText; -

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1753-1755 + // It so happens that CFGBlock::getTerminatorCondition returns 'A' for block + // B1, 'A && B' for B2, and 'A && B || C' for B3. Let's check whether we + // reached the end

[PATCH] D65212: [analyzer] Fix exporting SARIF files from scan-build on Windows

NoQ added inline comments. Comment at: clang/tools/scan-build/libexec/ccc-analyzer:121 END { - if (defined $ResultFile && -z $ResultFile) { unlink($ResultFile); Whoops. I suspect that we're now unconditionally deleting all plist output, which isn't exactl

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

NoQ added a comment. Looking good now! I still recommend eventually adding tests with casts of references. In D65889#1620128 , @Charusso wrote: > - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a > class. In D65889#1621587

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

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:94 + [CastFromName, CastToName, IsNullReturn, + IsSimpleCast](BugReport &) -> std::string { SmallString<128> Msg; `IsDynamicCast`.

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Aha, ok! Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; +

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

NoQ accepted this revision. NoQ edited reviewers, added: NoQ; removed: dergachev.a. NoQ added a comment. This will slightly skew Static Analyzer exploration order, not necessarily in a good way, as default arguments and (sometimes) default initializers are not modeled properly. But this doesn't

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

NoQ added a comment. Do you have commit access or should i commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65453/new/ https://reviews.llvm.org/D65453 ___ cfe-commits mailing list cfe

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

NoQ updated this revision to Diff 214441. NoQ added a comment. - Fix checker names for consumers that display them (eg., clang-tidy). Add a test. - Change bug descriptions: "Call to virtual function during construction or destruction" -> "Pure virtual method call" | "Unexpected loss of virtual

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ added a comment. In D64274#1600834 , @Szelethus wrote: > If either checker emits an error, the current implementation would say it > originates from `cplusplus.PureVirtualCall`. Could you please create a new > `ProgramPointTag` with the correct check

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

NoQ updated this revision to Diff 214462. NoQ retitled this revision from "[analyzer] WIP: Add fix-it hint support." to "[analyzer] Add fix-it hint support.". NoQ added a comment. - Hide all current fixits behind a checker option, have them off by default. They're not tested enough yet. - Suppre

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

NoQ updated this revision to Diff 214470. NoQ marked 8 inline comments as done. NoQ added a comment. Fxd! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65361/new/ https://reviews.llvm.org/D65361 Files: clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/test/Analysis/main.c clang

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:630 +(const RegionBindings::TreeTy *)((uintptr_t)store & ~(uintptr_t)1), +RBFactory.getTreeFactory(), (bool)((uintptr_t)store & (uintptr_t)1)); } xazax.hun wro

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

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:561 +CmdLineOption `Show` -> `Enable`? > **recursion**, //n.,// see "recursion" I think it's valuable when the object and the documentation describe the same idea in //different//

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

NoQ updated this revision to Diff 214495. NoQ marked 3 inline comments as done. NoQ added a comment. Fix un-singed options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/

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

NoQ added reviewers: alexfh, Szelethus. NoQ added a subscriber: alexfh. NoQ added a comment. +@alexfh because clang-tidy now finally has a way of safely disabling core checkers without causing crashes all over the place! Would you like to take the same approach as we picked in scan-build, i.e. w

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750 + // [B1] -> [B2] -> [B3] -> [sink] + // assert(A && B || C);\ \ \ + // ---> [go on with the executi

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

NoQ 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::vector

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. But i definitely like it how smooth this patch turned out to be! Also recent bugzilla requests for this feature: https://bugs.llvm.org/show_bug.cgi?id=42816 https://bugs.llvm.org/show_bug.cgi?id=418

[PATCH] D66045: Improve detection of same value in comparisons

NoQ added a reviewer: NoQ. NoQ added a comment. I like where this is going! I'll add myself so that not to forget to review the CFG bits next week. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66045/new/ https://reviews.llvm.org/D66045 __

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and therefore no range check is needed. Don't analyze further. + if (CE->getCastKind() ==

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

NoQ added a comment. 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 use the new feature when we know it'll do exactly what we want. It is going to be u

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

NoQ added a comment. In D66042#1624697 , @Charusso wrote: > `StringRef("osx.cocoa.RetainCountBase").startswith("osx.cocoa.RetainCount")` > is true, so there is no real issue until we manage the prefixes well. I > assume that the user who knows how to dis

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and therefore no range check is needed. Don't analyze further. + if (CE->getCastKind() ==

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

NoQ added a comment. In D65575#1625738 , @Szelethus wrote: > A little early, gentle ping :) I'm getting kinda paranoid with the size of > the stack, and how much I struggled with commiting last time. Mmm, what are your thoughts on my suggestions with wo

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

NoQ added a comment. In D66042#1625235 , @alexfh wrote: > In D66042#1624081 , @NoQ wrote: > > > +@alexfh because clang-tidy now finally has a way of safely disabling core > > checkers without causing crashes all ov

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

NoQ added a comment. In D66042#1625000 , @Szelethus wrote: > if we add this flag, people responsible for developing interafaces for the > analyzer might end up using it. And this is fine, that's supported. There's a very limited list of such people and

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great, thanks! I'd appreciate direct CFG tests for the part that changes the CFG (cf. `test/Analysis/cfg.cpp`), but i don't insist. Let's make sure @jfb is happy and commit :) =

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:1118 +Expr::EvalResult Result; +if (!Constant->EvaluateAsInt(Result, *Context)) + return {}; It's kinda strange to me that we first confirm that it's a constant by doing `tryTransformToIntO

[PATCH] D66045: Improve detection of same value in comparisons

NoQ added inline comments. Comment at: lib/AST/Expr.cpp:3931-3932 +case DeclRefExprClass: { + // DeclRefExpr without an ImplicitCastExpr can happen for integral + // template parameters. + const auto *DRE1 = cast(E1); Does this actually happen

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

NoQ added a comment. > 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 of having this patch landed is to allow users who are worried by false positives of sp

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

NoQ added a comment. Oh, these false positives! Thanks!! Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1627 +svalBuilder.makeIntVal(1, sizeTy), sizeTy); + Optional freeSpaceNL = freeSpace.getAs(); + Plea

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and therefore no range check is needed. Don't analyze further. + if (CE->getCastKind() ==

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

NoQ added a comment. 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 strong opinion against it and i don't want to push this further, but i just honestly thi

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

NoQ added a comment. In D66042#1627193 , @alexfh wrote: > Should this be different with clang and this patch? Nope. Moreover, this patch in fact introduces the same problem in scan-build :) In D66042#1627193 , @a

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

NoQ added a comment. In D66042#1626631 , @Szelethus wrote: > In D66042#1626513 , @Charusso wrote: > > > I really appreacite your ideas. It is unbelievable you guys bring up 20 > > different ideas for 5 LOC. I canno

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and theref

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

NoQ added a comment. In D66042#162 , @Szelethus wrote: > Yup. We will be able to present the drastic improvement made on LLVM > analyses, while giving us some breathing room to polish a long-term solution. > I suspect `-analyzer-config silence-checke

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

NoQ added a comment. In D66042#1627842 , @Charusso wrote: > @Szelethus, here is a really cool example: > https://clang.llvm.org/docs/ClangCommandLineReference.html. These are driver flags. They are indeed well-documented and user-facing. Frontend flags

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

NoQ added a comment. I'm convinced, let's keep everything as is but turn this into an `-analyzer-config` flag. We generally wanted to reduce the number of frontend flags because they are more taxing on backwards compatibility, so it makes perfect sense. CHANGES SINCE LAST ACTION https://rev

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

NoQ added a comment. Hmm, the patch doesn't pass its own test for me, could you double-check? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65453/new/ https://reviews.llvm.org/D65453 ___ cfe-commits ma

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

NoQ marked 2 inline comments as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:846 +print(' $ dot -Tsvg input.dot -o output.svg') +print() +write_temp_file('.dot', self.output()) --

[PATCH] D65349: [analyzer] Be more careful with destructors of non-regions.

NoQ updated this revision to Diff 214972. NoQ marked 2 inline comments as done. NoQ added a comment. Fxd! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65349/new/ https://reviews.llvm.org/D65349 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h clang/lib/Sta

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750 + // [B1] -> [B2] -> [B3] -> [sink] + // assert(A && B || C);\ \ \ + // ---> [go on with the executi

[PATCH] D66131: [analyzer] Don't track the condition of foreach loops

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Do it! 8) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66131/new/ https://reviews.llvm.org/D66131 ___

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Wish granted! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65287/new/ https://reviews.llvm.org/D65287 ___ cfe-commits mailing list cfe-commit

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

NoQ abandoned this revision. NoQ added a comment. Merged into D64274 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65180/new/ https://reviews.llvm.org/D65180 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ updated this revision to Diff 215000. NoQ added a comment. I'll merge D65180 into this patch because it's otherwise too painful to update. Like, D65180 untangles bug types and controls enabledness by initializing bug types,

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ updated this revision to Diff 215007. NoQ added a comment. Make the checkers independent and extract modeling into a dependency. This means that you can potentially enable the non-pure-virtual call checker without enabling the pure virtual call checker. This doesn't contradict backwards com

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

NoQ updated this revision to Diff 215011. NoQ added a comment. Rebase! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

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

NoQ accepted this revision. NoQ added a comment. Guys, thank you so much for working on establishing consensus on this change. //*bows*// CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 ___ cfe-commi

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

NoQ marked 3 inline comments as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:879 + ArrayRef getFixits() const { return Fixits; } + Szelethus wrote: > Hmm, will this return an immutable conta

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

NoQ 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 = +

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

NoQ added a comment. For me `ddd()` doesn't call `c::c()`. I can fix it by adding the following code: void VisitCXXConstructExpr(CXXConstructExpr *CE) { addCalledDecl(CE->getConstructor()); VisitChildren(CE); } I don't see why it would work without that code, as `CXXConstructExpr` is

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

NoQ added inline comments. Comment at: clang/lib/Analysis/CallGraph.cpp:93 void VisitCXXConstructExpr(CXXConstructExpr *E) { CXXConstructorDecl *Ctor = E->getConstructor(); Yes, there it is. Where does it come from? I don't have it in master. Reposito

<    7   8   9   10   11   12   13   14   15   16   >