[PATCH] D58065: [analyzer] Document the frontend library

george.karpenkov added a comment. High-level feedback: mixing of abstraction levels is wrong for the "bundled" documentation. This might also work better as a blogpost, if you want to jump from topic to topic. Comment at:

[PATCH] D57261: [analyzer] [WIP] Opt-in C Style Cast Checker for OSObject pointers

This revision was automatically updated to reflect the committed changes. Closed by commit rC353566: [analyzer] Opt-in C Style Cast Checker for OSObject pointers (authored by george.karpenkov, committed by ). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to

[PATCH] D54978: Move the SMT API to LLVM

george.karpenkov added a comment. > There is at least one other conflicting commit rL353465 > on top of this code already. Sorry about that. I think it would be fine to revert both or to replay the other commit on top of reverted version of this one.

[PATCH] D54978: Move the SMT API to LLVM

george.karpenkov added a comment. @mikhail.ramalho could you revert then? In general, we should not use Z3 unless it's explicitly requested. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54978/new/ https://reviews.llvm.org/D54978

[PATCH] D57782: [analyzer] [RetainCountChecker] Bugfix: in non-OSObject-mode, do not track CXX method calls

This revision was automatically updated to reflect the committed changes. george.karpenkov marked an inline comment as done. Closed by commit rC353227: [analyzer] [RetainCountChecker] Bugfix: in non-OSObject-mode, do not track CXX… (authored by george.karpenkov, committed by ). Herald added a

[PATCH] D57433: [analyzer] [RetainCountChecker] Bugfix for tracking top-level parameters of Objective-C methods

This revision was automatically updated to reflect the committed changes. Closed by commit rC352588: [analyzer] [RetainCountChecker] Bugfix for tracking top-level parameters of… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D57201: [analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's really doing two things

This revision was automatically updated to reflect the committed changes. Closed by commit rC352533: [analyzer] [RetainSummaryManager] [NFC] Split one function into two, as its… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D57344: Extend AnyCall to handle callable declarations without the call expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rC352531: Extend AnyCall to handle callable declarations without the call expressions (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D57346: [analyzer] [ARCMT] [NFC] Unify entry point into RetainSummaryManager

This revision was automatically updated to reflect the committed changes. Closed by commit rC352532: [analyzer] [ARCMT] [NFC] Unify entry point into RetainSummaryManager (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D57211: [analyzer] [RetainCountChecker] Support 'taggedRetain' and 'taggedRelease'

This revision was automatically updated to reflect the committed changes. Closed by commit rC352530: [analyzer] [RetainCountChecker] Support taggedRetain and taggedRelease (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

george.karpenkov added a comment. After this landed, options for RetainCountChecker stopped working - e.g. I can't use `osx.cocoa.RetainCount:blah=X`. Do you know why is this the case / how to fix it? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54438/new/

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

george.karpenkov added a comment. I personally do use HTML output quite a lot (and we do have non-Xcode projects), and complex macros in HTML are currently totally unusable. I'm not sure whether this is a right approach to handling this, but it's definitely a problem for us. In general - thanks

[PATCH] D57230: [analyzer] Toning down invalidation a bit

george.karpenkov added a comment. I'm in favor of this change, I never understood how invalidating a field invalidates entire structure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 ___

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

george.karpenkov added a comment. @thakis Sorry I don't understand your point. At the end of the day, ProgramPoint and BodyFarm are only used by the static analyzer. If you disagree, what is your proposed directory structure? What would be the benefits? Repository: rC Clang CHANGES SINCE

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

george.karpenkov added a comment. > Also, isn't lib/Analysis a strange place for this? lib/Analysis is used by > the compiler proper (CodeGen, Sema, …), while RetainSummaryManager is only > used by tooling-like things (static analyzer, arcmigrate). Not only, it has utilities used by the static

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

george.karpenkov added a comment. > Can you elaborate a bit in what sense this decouples ARCMT from the analyzer? ARCMT now does not need to link against the analyzer > Can CLANG_ENABLE_STATIC_ANALYZER now be set independently of > CLANG_ENABLE_ARCMT? Yes Repository: rC Clang CHANGES

[PATCH] D57204: [AST] Add a method to get a call type from an ObjCMessageExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rC352147: [AST] Add a method to get a call type from an ObjCMessageExpr (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D57062: [analyzer] Re-enable the "System is over constrained" assertion on optimized builds.

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. We would start getting crashes from `TrustNonnullChecker` if we enable it. Shouldn't those be fixed first? An input which crashes with this assertion is

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

george.karpenkov added a comment. > Hmmm, does this mess with options that bad? Could you please clarify? `registerChecker` gets-or-creates a checker object. A checker name (used for getting the options) is set the first time it's created. The checker which was created first "wins" and gets to

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

george.karpenkov added a comment. > Deal with the consequences of this, and just correct all plist files to now > refer to the new base checker. What does it mean? > Each time a report is emitted from these checkers, create a ProgramPointTag, > and "manually" make sure the emitted checker

[PATCH] D56899: [analyzer] pr37688: Fix a crash on trying to evaluate a deleted destructor of a union.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. The code is fine, but I obviously would prefer a proper fix in a CFG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56899/new/

[PATCH] D56891: [analyzer] Introduce proper diagnostic for freeing unowned object

This revision was automatically updated to reflect the committed changes. Closed by commit rC351514: [analyzer] Introduce proper diagnostic for freeing unowned object (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D56885: [analyzer] const-ify reference to bug type used in BugReporter

This revision was automatically updated to reflect the committed changes. Closed by commit rC351511: [analyzer] const-ify reference to bug type used in BugReporter (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D56884: [analyzer] Extend BugType constructor to accept "SuppressOnSink" as a parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rC351510: [analyzer] Extend BugType constructor to accept SuppressOnSink as a parameter (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D56820: [analyzer] [RetainCountChecker] Produce a correct message when OSTypeAlloc is used

This revision was automatically updated to reflect the committed changes. Closed by commit rC351509: [analyzer] [RetainCountChecker] Produce a correct message when OSTypeAlloc is… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D56823: [analyzer] Do not try to body-farm bodies for Objective-C properties with custom accessors.

george.karpenkov added a comment. LG, but it sounds like something which can skew results a lot (?) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56823/new/ https://reviews.llvm.org/D56823 ___ cfe-commits mailing

[PATCH] D56759: [analyzer] Another RetainCountChecker cleanup

This revision was automatically updated to reflect the committed changes. Closed by commit rC351394: [analyzer] Another RetainCountChecker cleanup (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

This revision was automatically updated to reflect the committed changes. Closed by commit rL351097: [analyzer] [PR39792] false positive on strcpy targeting struct members (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

george.karpenkov added a comment. @Pierre-vh The patch does not compile due to unmatched braces. Please do test and compile before submitting! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 ___

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

george.karpenkov added a comment. Whoops, sorry. There were holidays, and then I did forget about this patch. I'll commit this now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 ___ cfe-commits

[PATCH] D55616: Emit ASM input in a constant context

george.karpenkov added a comment. Apologies - the value seems to indeed overflow, but I'm still very confused how this was affected by this change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55616/new/ https://reviews.llvm.org/D55616

[PATCH] D55616: Emit ASM input in a constant context

george.karpenkov added a comment. @void @efriedma I can't build XNU anymore after this commit, with an error message: osfmk/i386/genassym.c:298:2: error: value '18446743523953737728' out of range for constraint 'n' DECLAREULL("KERNEL_BASE", KERNEL_BASE);

[PATCH] D56402: [analyzer] [NFC] [RetainCountChecker] Remove dead unused map

This revision was automatically updated to reflect the committed changes. Closed by commit rC350868: [analyzer] [NFC] [RetainCountChecker] Remove dead unused map (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

george.karpenkov accepted this revision. george.karpenkov added a comment. Seems good, but a comment explaining why this is necessary and why the crash follows otherwise would be great. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55875/new/ https://reviews.llvm.org/D55875

[PATCH] D55978: [gn build] Add build files for clang/lib/{ASTMatchers, CrossTU}, clang/lib/StaticAnalyzer/{Checkers, Core, Frontend}

george.karpenkov added a comment. > I might want to default to clang_enable_static_analyzer=false when I add the > clang/lib/FrontendTool build file. I don't think that quite makes sense, since by default clang does have the analyzer built in. CHANGES SINCE LAST ACTION

[PATCH] D55978: [gn build] Add build files for clang/lib/{ASTMatchers, CrossTU}, clang/lib/StaticAnalyzer/{Checkers, Core, Frontend}

george.karpenkov added a comment. Looks reasonable, what about linking with Z3? Or is your goal just to get a minimally working functionality? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55978/new/ https://reviews.llvm.org/D55978 ___

[PATCH] D55976: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions

This revision was automatically updated to reflect the committed changes. Closed by commit rC349876: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55907: [analyzer] RetainCount: Bluntly suppress the CFRetain detection heuristic on a couple of CM functions.

george.karpenkov added a comment. In general, I'm against hardcoding, and I think we should use the annotations indicating that the function does retain. (or use a naming convention, and use an annotation to indicate that the function does not retain). However, if this is prohibitive for some

[PATCH] D55823: [analyzer] Fix backward compatibility issue after D53280 'Emit an error for invalid -analyzer-config inputs'

george.karpenkov added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3700 + // through them. + for (size_t Index = 0; Index < Args.size(); ++Index) { +if (StringRef(Args.getArgString(Index)).contains("-analyzer-config")) { NoQ wrote: > Needs

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

george.karpenkov added a comment. This is amazing, huge thanks for doing this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55792/new/ https://reviews.llvm.org/D55792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

george.karpenkov added a comment. @NoQ thanks, interesting! But I thought the underlying map is not actually modified while the reference is alive? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54921/new/ https://reviews.llvm.org/D54921

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rC348822: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55528: [analyzer] Resolve another bug where the name of the leaked object was not printed properly

This revision was automatically updated to reflect the committed changes. Closed by commit rC348819: [analyzer] Resolve another bug where the name of the leaked object was not… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55465: Stop tracking retain count of OSObject after escape to void * / other primitive types

This revision was automatically updated to reflect the committed changes. Closed by commit rC348675: Stop tracking retain count of OSObject after escape to void * / other primitive… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55385: [analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine

This revision was automatically updated to reflect the committed changes. Closed by commit rC348637: [analyzer] RetainCountChecker: remove untested, unused, incorrect option… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

This revision was automatically updated to reflect the committed changes. Closed by commit rC348638: [analyzer] Move out tracking retain count for OSObjects into a separate checker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55158: [analyzer] Rely on os_consumes_this attribute to signify that the method call consumes a reference for "this"

This revision was automatically updated to reflect the committed changes. Closed by commit rC348533: [analyzer] Rely on os_consumes_this attribute to signify that the method call… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55155: [attributes] Add an attribute "os_consumes_this" with similar semantics to "ns_consumes_self"

This revision was automatically updated to reflect the committed changes. Closed by commit rC348532: [attributes] Add an attribute os_consumes_this, with similar semantics to… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

george.karpenkov added a comment. > Well,, that seems unfortunate if we have the only supported interface for the > static analyzer be an internal interface. Perhaps it can be given a different > option? Even discounting this change, I that seems like it would be > appropriate. Officially

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

george.karpenkov added reviewers: dcoughlin, NoQ. george.karpenkov requested changes to this revision. george.karpenkov added a comment. Using `-Xclang` is the only way to pass options to the static analyzer, I don't think we should warn on it. CHANGES SINCE LAST ACTION

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55321/new/ https://reviews.llvm.org/D55321 ___ cfe-commits

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thank you for the fix, but how far can the pattern matching go? Seems easy enough to think of cases not covered by the above. In any case, the fix looks good. Repository:

[PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

george.karpenkov marked an inline comment as done. george.karpenkov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp:483-497 case CE_Function: Summ = getFunctionSummary(cast(Call).getDecl()); break; case CE_CXXMember:

[PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

This revision was automatically updated to reflect the committed changes. Closed by commit rC347949: [analyzer] RetainCountChecker: recognize that OSObject can be created directly… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55041: [analyzer] Switch retain count checker for OSObject to use OS_* attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rC347948: [analyzer] Switch retain count checker for OSObject to use OS_* attributes (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D55033: [analyzer] Add the type of the leaked object to the diagnostic message

This revision was automatically updated to reflect the committed changes. Closed by commit rC347943: [analyzer] Add the type of the leaked object to the diagnostic message (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

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

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: test/Analysis/uninit-vals.m:222 testObj->origin = makeIntPoint(1, 2); - if (testObj->size > 0) { ; } //

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

george.karpenkov added a comment. This looks more reasonable, thanks! What about just dropping the `Knowing` prefix? Just having `arr is null, taking true branch` seems considerably more readable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53076/new/ https://reviews.llvm.org/D53076

[PATCH] D54457: [AST] Generate unique identifiers for CXXCtorInitializer objects.

george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: lib/AST/DeclCXX.cpp:2249 +int64_t CXXCtorInitializer::getID(const ASTContext ) const { + Optional Out =

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

george.karpenkov added a comment. In the future, for macOS-specific changes I think it would be better to wait for a sign-off from at least one maintainer who is an expert on Apple tools. Repository: rL LLVM https://reviews.llvm.org/D54310 ___

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

george.karpenkov added a comment. I suspect it broke http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/14090/console as well. Repository: rL LLVM https://reviews.llvm.org/D54310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

george.karpenkov added a comment. I don't quite understand the need for this patch. If we are talking about a binary built from source, wouldn't it make more sense to build libcxx from source as well? If libcxx is built from source in the same repo, clang-based tools do find it. Repository:

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

george.karpenkov added a comment. From a first glance, it's missing screenshots and an introduction section. What do you propose we should do with other pages on the existing website? (OpenProjects / etc.) Repository: rC Clang https://reviews.llvm.org/D54429

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

george.karpenkov added a comment. > Also, remove diags::note_suggest_disabling_all_checkers. Isn't that a separate revision? Given that removing existing options is often questionable, I'd much rather see this patch separated. Repository: rC Clang https://reviews.llvm.org/D54401

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

george.karpenkov added inline comments. Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:177 "no analyzer checkers are associated with '%0'">; -def note_suggest_disabling_all_checkers : Note< -"use -analyzer-disable-all-checks to disable all static analyzer

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:8 +// recursive template instantiation limit. +// XFAIL: darwin && asan +

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

george.karpenkov added inline comments. Comment at: clang/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:8 +// recursive template instantiation limit. +// XFAIL: darwin && asan + Do we actually want UNSUPPORTED here? We don't want to fail if ASAN stack

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

george.karpenkov added a subscriber: vsapsai. george.karpenkov added a comment. @lebedev.ri yeah ASAN is making stack frame size larger. It seems @vsapsai is working on disabling this test under ASAN. Repository: rC Clang https://reviews.llvm.org/D50050

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

george.karpenkov added a comment. @lebedev.ri @erichkeane The test fails for me on macOS whenever asan and ubsan are both enabled. The failure is stack overflow at stack frame 943 (? maybe asan usage enforces lower stack size?) Repository: rC Clang https://reviews.llvm.org/D50050

[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.

george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369 ProgramStateRef state = C.getState(); - RegionStateTy RS = state->get(); + RegionStateTy OldRS = state->get(); RegionStateTy::Factory = state->get_context();

[PATCH] D53277: [analyzer][NFC] Collect all -analyzer-config options in a .def file

george.karpenkov added a comment. @NoQ @Szelethus Actually pushed a fix. Repository: rL LLVM https://reviews.llvm.org/D53277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53277: [analyzer][NFC] Collect all -analyzer-config options in a .def file

george.karpenkov added a comment. @Szelethus @NoQ OK it seems you need to annotate those headers in modulemap. E.g. take a look at: george1@/Volumes/Transcend/code/monorepo/llvm-project/clang (master)≻ rg SVals.def include/clang/module.modulemap 131: textual header

[PATCH] D53277: [analyzer][NFC] Collect all -analyzer-config options in a .def file

george.karpenkov added a comment. @Szelethus you can see the command required to get modules build in the mentioned log. I think -DLLVM_ENABLE_MODULES=On might be enough. Repository: rL LLVM https://reviews.llvm.org/D53277 ___ cfe-commits

[PATCH] D53982: Output "rule" information in SARIF

george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif:10 }, - "length": 510, +

[PATCH] D53982: Output "rule" information in SARIF

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor nit: let's create a custom substitution for your diff command, like `diff_plist`. Otherwise LGTM. Comment at:

[PATCH] D53902: RetainCountChecker: for now, do not trust the summaries of inlined code

This revision was automatically updated to reflect the committed changes. Closed by commit rC345746: [analyzer] RetainCountChecker: for now, do not trust the summaries of inlined… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D53628: [analyzer] Remove custom rule for OSIterator in RetainCountChecker

george.karpenkov added a comment. Actually, I'm reverting this: the rule seems fairly ubiquitous. Repository: rC Clang https://reviews.llvm.org/D53628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53814: Allow the analyzer to output to a SARIF file

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. I much prefer this version. We've had the same problem with diffing plist output. One thing we have learned is using a FileCheck was definitely a bad idea, as it leads to

[PATCH] D53814: Allow the analyzer to output to a SARIF file

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. I don't think a new PathGenerationScheme is needed, unless you plan changes to BugReporter.cpp. The code is fine otherwise, but could we try to use `diff` for

[PATCH] D53814: Allow the analyzer to output to a SARIF file

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor style notes + context missing. I think using `diff` would be better than a custom python tool. https://reviews.llvm.org/D53814

[PATCH] D53814: Allow the analyzer to output to a SARIF file

george.karpenkov added a comment. Patch context is missing. Comment at: Analysis/diagnostics/sarif-check.py:22 +passes = 0 +with open(testfile) as testfh: +lineno = 0 Wow, this is super neat! Since you are quite active in LLVM community, would you think

[PATCH] D53628: [analyzer] Remove custom rule for OSIterator in RetainCountChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rC345339: [analyzer] Remove custom rule for OSIterator in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

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

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Actually, apologies again: I have rushed through this too much. @NoQ has a good point: we need to preserve the distinction between the things analyzer "assumes"

[PATCH] D53276: [analyzer][NFC] Fix some incorrect uses of -analyzer-config options

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor nitpicks, otherwise makes sense! Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:746 + /// exploring a top level

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

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Thanks a lot, this is almost ready! I think it should be good to go after this round of nitpicks. Comment at:

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

george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: test/Analysis/self-assign.cpp:42 str = rhs.str; - rhs.str = nullptr; // FIXME: An improved leak checker should warn here + rhs.str =

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

george.karpenkov added a comment. I'll try to take a look this week. 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/cfe-commits

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

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:80 typedef llvm::ImmutableMap GenericDataMap;

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

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Good to go provided you will add an example. If we want to be serious about this page, it really has to be auto-generated (like clang-tidy one), but I understand

[PATCH] D53615: [analyzer] [NFC] Change scanReachableSymbols to use ranges

This revision was automatically updated to reflect the committed changes. Closed by commit rC345101: [analyzer] [NFC] Change scanReachableSymbols to use ranges (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D53550: [analyzer] Do not stop tracking CXX methods touching OSObject.

This revision was automatically updated to reflect the committed changes. Closed by commit rC345100: [analyzer] Do not stop tracking CXX methods touching OSObject. (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D53549: [analyzer] Trust summaries for OSObject::retain and OSObject::release

This revision was automatically updated to reflect the committed changes. Closed by commit rC345099: [analyzer] Trust summaries for OSObject::retain and OSObject::release (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

george.karpenkov added a comment. > because the guard that prevents it from working is useless and can be removed > as well Should we remove it then? Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:729 + SourceLocation L = FD->getLocation(); + return !L.isValid()

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

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:202 + void finalizeConstraints() { +

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

george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:80 class ConstraintManager { + using ConstraintMap = std::map; + ConstraintMap Constraints; Traditionally LLVM projects use

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

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:

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

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

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

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

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

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 -

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

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:

  1   2   3   4   5   6   >