[PATCH] D43840: [CFG] [analyzer] Fix a crash on finding construction context for implicit constructor conversion.

2018-02-28 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Should there be a FIXME note saying that other casts should be supported? Repository: rC Clang https://reviews.llvm.org/D43840 ___

[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?

2018-02-28 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Currently there's no test demonstrating a different behavior from `cfg-temporary-dtors` being set to true?.. Repository: rC Clang https://reviews.llvm.org/D43804 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D43925: [analyzer] Prevent crashing in NonNullParamChecker

2018-03-01 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326520: [analyzer] Prevent crashing in NonNullParamChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D43925 F

[PATCH] D44044: [analyzer] Don't throw NSNumberObjectConversion warning on object initialization in if-expression

2018-03-02 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326619: [analyzer] Don't throw NSNumberObjectConversion warning on object… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.ll

[PATCH] D44059: [analyzer] AST-matching checker to detect global central dispatch performance anti-pattern

2018-03-05 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326746: [analyzer] AST-matching checker to detect global central dispatch performance… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https:

[PATCH] D44124: [analyzer] Support destruction and lifetime-extension of inlined function return values.

2018-03-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:316 } } break; 5 closing braces is a lot. What about moving the entire block under `CK_Complete` into a static function? Repository: rC Clang http

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2018-03-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:618 + StringRef Name = FD->getIdentifier()->getName(); + int ArgIndex = llvm::StringSwitch(Name) + .Case("sprintf", 1) koldaniel wrot

[PATCH] D44169: [ASTMatcher] Extend hasAnyArgument to ObjCMessageExpr

2018-03-06 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326865: [ASTMatcher] Extend hasAnyArgument to ObjCMessageExpr (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D44

[PATCH] D44169: [ASTMatcher] Extend hasAnyArgument to ObjCMessageExpr

2018-03-06 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL326865: [ASTMatcher] Extend hasAnyArgument to ObjCMessageExpr (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4

[PATCH] D44170: [analyzer] Update GCDAsyncSemaphoreChecker to match messages send to ObjC objects

2018-03-06 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326868: [analyzer] Fix the checker for the performance anti-pattern to accept messages (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https:

[PATCH] D43917: [analyzer] [NFC] Minor refactoring of NonNullParamChecker

2018-03-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326935: [analyzer] [NFC] Minor refactoring of NonNullParamChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D4

[PATCH] D44183: [analyzer] Don't crash with assertion failure on structured bindings

2018-03-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326951: [analyzer] Don't crash with assertion failure on structured bindings (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://revie

[PATCH] D44172: [analyzer] [PointerArithChecker] do not warn on indexes into vector types

2018-03-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326952: [analyzer] [PointerArithChecker] do not warn on indexes into vector types (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://rev

[PATCH] D44131: [analyzer] Support temporaries conjured by conservatively evaluated functions.

2018-03-07 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D44131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[PATCH] D44178: [analyzer] Correctly model iteration through "nil" objects

2018-03-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326982: [analyzer] Correctly model iteration through "nil" objects (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D44281: [analyzer] Suppress more MallocChecker positives in reference counting pointer destructors.

2018-03-09 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > we often run out of inlining stack depth limit Can we consider increasing that limit? I'd much rather have a limit on maximum path *length* (which we currently don't have), as longer paths are more likely to be false positives. On the other hand, I don't see

[PATCH] D44228: [analyzer] Move the GCDAsyncSemaphoreChecker to optin.performance

2018-03-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC327309: [analyzer] Move the GCDAsyncSemaphoreChecker to optin.performance (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.

[PATCH] D44409: [analyzer] Fix crashes in RetainCountChecker when underlying region is not a var

2018-03-16 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC327727: [analyzer] Fix crashes in RetainCountChecker when underlying region is not a var (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: h

[PATCH] D44594: [analyzer] Fix the assertion failure when static globals are used in lambda by reference

2018-03-19 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC327926: [analyzer] Fix the assertion failure when static globals are used in lambda by… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: ht

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2382 +// Reset the solver +RefutationMgr.reset(); + } xazax.hun wrote: > george.karpenkov wrote: > > george.karpenkov wrote: > > > (apologies in advance f

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Herald added subscribers: a.sidorin, zzheng, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. @ddcc Hi, are you still interested in landing the fixes associated with this patch? I can take a look as I'm currently reviewing https://reviews.llvm.org/D4

[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. requesting changes due to minor nits inline Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:152 +} + } + void Profile(llvm::FoldingSetN

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2382 +// Reset the solver +RefutationMgr.reset(); + } xazax.hun wrote: > george.karpenkov wrote: > > xazax.hun wrote: > > > george.karpenkov wrote: > > >

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @xazax.hun (I'll reply here to avoid scattering the conversation across many subtrees) I was thinking about the optimization for not adding redundant constraints some more, and I've decided I'm still against it --- we are creating a higher potential for bugs, a

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > I am not not sure that I got the idea what are you suggesting here. If we > have the constraint of for example a symbol s > 10 and later on a path we > discover s > 20, will we also deduplicate this that way? No. But I thought in your optimization atoms insid

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @trixirt I wrote a bunch of comments inline, but then after looking at the test cases, this would not work. This is a dataflow, all-paths check (the variable can NEVER be a nullptr), yet symbolic execution runs on A given paths. Currently in the analyzer

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @xazax.hun > So strictly speaking it is not a deduplication on the constraint level but on > the symbol level. Right, apologies, I was initially mistaken then. That's not even deduplication, I would call it using the interval solver to guide the constraint sel

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > I can also split out the APSInt fix into a separate patch. Yes please. In general I really dislike arbitrary "complexity cutoffs", as they make further debugging hard and may lead to weird bugs: could you explain why is that required and can not be avoided?

[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM, but I would really prefer if you could split that inheritance back into composition. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123 +/// AS

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. LGTM with a nit to me, but maybe clang-tidy code owners would need to sign that off as well. Comment at: clang-tidy/ClangTidy.h:232 + bool EnableCheckProfile = false, +

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @Szelethus I personally really like this idea, and I think it would be a great checker. Could you perform more evaluation on other projects? https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@

[PATCH] D41150: [CFG] Adding new CFGStmt LoopEntrance for the StaticAnalyzer

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I would prefer a more detailed CFG element with more meta-information about nested loop, but this is a good starting point. https://reviews.llvm.org/D41150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D41150: [CFG] Adding new CFGStmt LoopEntrance for the StaticAnalyzer

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Also loops created with gotos should also be supported (e.g. by finding backedges), but that could be done in a different patch. https://reviews.llvm.org/D41150 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D44238: [CFG] Fix automatic destructors when a member is bound to a reference.

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. I'm not an expert in CFG construction, but looks good to me. https://reviews.llvm.org/D44238 ___ cfe-commits mailing list cfe

[PATCH] D39398: [CFG][Analyzer] Add LoopExit element to the CFG in more cases

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Herald added a subscriber: dmgreen. Comment at: lib/Analysis/CFG.cpp:1348 +for (auto &Parent : ASTCtx.getParents(Node)) { + NodesTo

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Herald added a subscriber: baloghadamsoftware. The change LGTM after nits and rebasing IF the new evaluation will not show a too large regression. =

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

2018-05-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. @ddcc so would be great if we could split this patch into smaller chunks. https://reviews.llvm.org/D35450 ___ cfe-c

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Thanks! Looks good with minor changes. Would it be possible to add tests? I know we have very few unit tests, but I assume you could actually use an integration t

[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. 👍 https://reviews.llvm.org/D47350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang https://reviews.llvm.org/D47603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Thanks, this is going in the right direction! Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:182 + virtual

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:928 + bool isModelFeasible() override { +return Solver.check() != Z3_L_FALSE; + } george.karpenkov wrote: > The semantics of this method is incorrect. It sho

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:925 + void reset() override { Solver.reset(); } + mikhail.ramalho wrote: > george.karpenkov wrote: > > We don't need `reset` anymore. > We don't need it but the

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-05-31 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:183 + virtual void reset() {} + virtual bool isModelFeasible() { return true; } + virtual void addRangeConstraints(ConstraintRangeTy) {} mi

[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)

2018-06-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. LGTM with a nit on a test name. Comment at: test/Analysis/pr37646.c:1 +// REQUIRES: z3 +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9

[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)

2018-06-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Thanks! Repository: rC Clang https://reviews.llvm.org/D47617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47616: [CFG] [analyzer] Explain copy elision through construction contexts.

2018-06-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. There's quite a lot of code duplication here, I think we could do better with that. Great job modeling semantics though! Comment at: lib/Analys

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-06-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @mikhail.ramalho I assume you know it, but just in case, you can mark dependencies in phabricator by adding "parent" revisions. https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-06-03 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. > I updated the test case as the cross check is not marking the true bug as > invalid anymore. Awesome! Does it mean that the optimization for adding less constra

[PATCH] D45517: [analyzer] False positive refutation with Z3

2018-06-03 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > I pretty sure it was not related to the optimizations, I removed them days ago (in the previous version of this patch) and the bug was still there. OK so any idea what the change could have been? Clearly the bug was there but not now. Anyway, should be OK to

[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)

2018-06-04 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang https://reviews.llvm.org/D47617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

2018-06-04 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Impressive, really. So almost all tests still pass. However, a few questions should be resolved before we can merge: - Do all tests for Z3 run when LLVM is configured to use Z3? I'm not sure if that's the right thing: do all tests start to take 10x time to run o

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

2018-06-04 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Seems to be good to merge regardless. Repository: rC Clang https://reviews.llvm.org/D47726 ___ cfe-commits mailing list cf

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

2018-06-04 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a subscriber: mikhail.ramalho. george.karpenkov added a comment. @ddcc To be completely honest, I see a few design issues with the current implementation of Z3 backend, the main one being that it checks satisfiability after every single exploded node. To the best of my kn

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

2018-06-04 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > I agree, though a number of these are limitations in CSA, and not > specifically the backend. Yeah, so for instance we always assume that for a given state we know whether it's feasible or not, and IMO for efficient SMT solver support we would need to operate

[PATCH] D47671: [analyzer] Implement copy elision.

2018-06-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > So having an analyzer-config option would be useful for those codebases that > are expected to be compiled with less advanced compilers that do not elide > copies or not doing it aggressively enough. I'm not sure it makes sense. From my understanding, that's

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

2018-06-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89 +new Callback(LCtx, MRMgr, ITraits)); + Finder.matchAST(ASTCtx); + IMO using the iterator directly (e.g. like it was done in https://github.com/l

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

2018-06-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89 +new Callback(LCtx, MRMgr, ITraits)); + Finder.matchAST(ASTCtx); + ormris wrote: > george.karpenkov wrote: > > IMO using the iterator directly (e.

[PATCH] D47756: [analyzer] [NFC] Unify Minimal and Extensive diagnostics.

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC334525: [analyzer] [NFC] Unify Minimal and Extensive diagnostics. (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org

[PATCH] D47808: [analyzer] [NFC] Now let's have only one place for diagnostics generation

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC334526: [analyzer] [NFC] Now let's have only one place for diagnostics generation (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://

[PATCH] D48042: [analyzer] Replace most usages of getEndPath

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC334540: [analyzer] [NFC] Remove most usages of getEndPath (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D48042?

[PATCH] D48035: [analyzer] [NFC] Move ::dump methods from BugReporter.cpp to PathDiagnostics.cpp

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC334541: [analyzer] [NFC] Move ::dump methods from BugReporter.cpp to PathDiagnostics.cpp (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D48045: [analyzer] [NFC] Remove "removeInvalidation" from visitor API

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334542: [analyzer] [NFC] Remove "removeInvalidation" from visitor API (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D48045: [analyzer] [NFC] Remove "removeInvalidation" from visitor API

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC334542: [analyzer] [NFC] Remove "removeInvalidation" from visitor API (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Looks good to me, apart from the nit on an unused header. Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:21 #include "clang/StaticAnalyzer/Core/Pat

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

2018-06-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added subscribers: aaron.ballman, alexfh. george.karpenkov added a comment. Herald added a subscriber: mikhail.ramalho. Getting there! Sorry for the delay. Would it be possible to rewrite the remaining imperative code using matchers? 👍 on defining your own matcher. However, that

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

2018-06-13 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > This change has been committed, so I'm closing this review. @ormris The process which should be followed here is to add a line (exactly) "Differential Revision: https://reviews.llvm.org/D47044"; to the bottom of your commit message, so that the phabricator ca

[PATCH] D48205: [analyzer] Assert that nonloc::SymbolVal always wraps a non-Loc-type symbol.

2018-06-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1241 if (const llvm::APSInt *I = - SVB.getKnownValue(State, nonloc::Symb

[PATCH] D48204: [analyzer] Make getDerefExpr() skip cleanups.

2018-06-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: test/Analysis/inlining/inline-defensive-checks.cpp:93 +S *conjure(); +S *get_conjured(S _) { + S *s = conjure(); what is the argument doing? Repository: rC Clang https://reviews.llvm.org/D48204

[PATCH] D48285: [analyzer]{UninitializedObjectChecker] Added "NotesAsWarnings" flag

2018-06-18 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM provided the nit on passing arguments is fixed. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:696 void ento::registerUninit

[PATCH] D47670: [analyzer] [NFC] Remove unused Extensive diagnostic setting, rename AlternateExtensive to Extensive.

2018-06-18 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @alexfh apologies, fixed. Repository: rC Clang https://reviews.llvm.org/D47670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name

2018-06-18 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Looks good overall, but some nits inline. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:682 +StringRef getVariableNam

[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name

2018-06-19 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:685 + + if (CXXParent && CXXParent->isLambda()) { +CXXRecordDecl::capture_const_iterator CapturedVar = Szelethus wrote: > george.karpenkov wrote: >

[PATCH] D45718: [analyzer] Allow registering custom statically-linked analyzer checkers

2018-06-21 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D45718 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D52905: [analyzer] fix accessing GDM data from shared libraries

2018-10-09 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > there's a way to run clang-tidy over all of clang+llvm automatically cmake should generate compile_commands.json by default, and then you could just point clang-tidy at that. Repository: rC Clang https://reviews.llvm.org/D52905

[PATCH] D53076: [analyzer] WIP: Enhance ConditionBRVisitor to write out more information

2018-10-10 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. The change makes sense to me, but: 1. Note that "Assuming X" directives are useful for the analyzer developers, since they know that's where the checker makes arbitrary assumptions, but to end users that mostly feels like noise ("Taking true branch" is there al

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

2018-10-10 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Herald added a subscriber: donat.nagy. Great idea, thanks! Should be good to go once examples are added, and implicit checks are removed. Comme

[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-10-10 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Herald added a subscriber: donat.nagy. @Szelethus @xazax.hun Since you guys are looking into the website, do you know why the top bar has disappeared from all pages? (E.g. http://clang-analyzer.llvm.org/available_checks.html ) https://reviews.llvm.org/D52984

[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-10-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > inline the header into every page, which is horrible is it though? we probably also should be moving towards an auto-generated model anyway, if we want to e.g. automatically build sections of a website from the code. https://reviews.llvm.org/D52984 _

[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-10-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > This too will most likely require server maintainers to intervene, so i think > fixing SSI is more realistic. Not really. The way it is done e.g. for http://clang.llvm.org/docs/LibASTMatchersReference.html is the HTML files are in the repo, there is a script

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @Szelethus I take it this is mostly formed from @NoQ email? Language could use polishing in quite a few places, could I just commandeer this revision and try to fix it? Comment at: www/analyzer/open_projects.html:29 +Implement a dataflow

[PATCH] D53274: [analyzer][NFC] Fix inconsistencies in AnalyzerOptions

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > The main motivation behind here is to emit warnings if an invalid I'm totally with you here, but IIRC (@NoQ might want to correct me here), the design decision was made specifically to ignore incorrect options, so that e.g. old versions of Xcode used with old

[PATCH] D53274: [analyzer][NFC] Fix inconsistencies in AnalyzerOptions

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I'm not sure why you could get away with removing those llvm_unreachable cases? Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:373 - default: -llvm_unreachable("Invalid mode."); case UMK_Shallow: Wh

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I have tried to clean up the list. https://reviews.llvm.org/D53024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov updated this revision to Diff 169758. https://reviews.llvm.org/D53024 Files: clang/www/analyzer/open_projects.html Index: clang/www/analyzer/open_projects.html === --- clang/www/analyzer/open_projects.html +++ cla

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. +1, I think this is a great addition! BTW in theory it should be possible to go wild on compile-time programming, and generate the entire description string at co

[PATCH] D53274: [analyzer][NFC] Fix inconsistencies in AnalyzerOptions

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. OK that makes sense to me, just let's be careful not to crash in places where we weren't crashing before. Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:354 +StringRef AnalyzerOptions::getOptionAsString(Optional &V, +

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-15 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @Szelethus thanks! BTW if you really want to invest into maintaining the website, I think it's totally worth it to change all contents to markdown, and then have a script to generate HTML from that. Committers would be expected to manually run that script. This w

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

2018-10-16 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677 +/// need to expanded further when it is nested inside another macro. +class Ma

[PATCH] D52790: [analyzer][PlistMacroExpansion] New flag to convert macro expansions to events

2018-10-16 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:6 clang_version -clang version 8.0.0 (http://mainstream.inf.elte.hu/Szelethus/clang 80e1678b9f598ca78bb3b71cf546a63414a37b11) (https://github.com/llv

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-16 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Herald added a subscriber: dkrupp. Comment at: clang/www/analyzer/open_projects.html:27-32 +New checkers which were contributed to the analyzer, +but have not passed a rigorous evaluation process, +are committed as "alpha checke

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-16 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov updated this revision to Diff 169920. https://reviews.llvm.org/D53024 Files: clang/www/analyzer/open_projects.html Index: clang/www/analyzer/open_projects.html === --- clang/www/analyzer/open_projects.html +++ cla

[PATCH] D53024: [analyzer][www] Add more open projects

2018-10-16 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344663: [analyzer] [www] Updated a list of open projects (authored by george.karpenkov, committed by ). Changed prior to commit: https://reviews.llvm.org/D53024?vs=169920&id=169933#toc Repository: rC

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

2018-10-16 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Herald added a subscriber: dkrupp. @Szelethus Also you have without a doubt noticed that a "Download" section on the index page could be improved :P https://reviews.llvm.org/D53069 ___ cfe-commits mailing list cfe-

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Good to go, with minor nits inline. Comment at: lib/FrontendTool/ExecuteCompilerInvocation.cpp:255 +ento::printAnalyzerConfigList(llvm::outs()); +

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: www/analyzer/available_checks.html:459 + Spurious newline Comment at: www/analyzer/avail

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Herald added subscribers: dkrupp, donat.nagy. I'm marking it as "needs changes" for now -- the idea seems reasonable, but it's hard to tell without a thorough evaluation on at least a few codebases. Additionally, we should figure out a checker group for this - if

[PATCH] D53483: [analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. OK, so the overall direction makes sense: unregistered options are restricted to checkers, and for others, an explicit getter must be maintained. (though I would also prefer if even checkers could pre-register their options somehow) @NoQ does this make sense to

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @bcraig comments look valid, marking as "Needs Changes" Repository: rC Clang https://reviews.llvm.org/D53206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D52844: [analyzer] [testing] Compute data on path length, compute percentiles

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344990: [analyzer] [testing] Compute data on path length, compute percentiles (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://revi

<    1   2   3   4   5   6   >