[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

hgabii marked 2 inline comments as done. hgabii added a comment. @steakhal I think it is basically done. If you have any recommendations what needs to be changed, then I welcome your contribution. I can also fix new comments, but I have limited time for this. Repository: rCTE Clang Tools Ext

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

xazax.hun accepted this revision. xazax.hun added a comment. LG! It is an interesting idea to use this facility for `trackExpressionValue`. But I would expect such a mechanism to trigger quite often. I wonder if the memory consumption would increase significantly by storing a lambda for almost

[PATCH] D57860: [analyzer] Validate checker option names and values

xazax.hun added a comment. Did we test all the codepaths including the package level configs? If not, please add some package level config option related tests. Otherwise the patch looks good to me after all the comments are resolved. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57860/

[PATCH] D59457: [analyzer][NFC] Use capital variable names in CheckerRegistry

xazax.hun added a comment. I did not check the patch yet but wanted to point out that we might not want to rush about renaming all the variables until the community decides on the coding guideline, see https://reviews.llvm.org/D59251 Repository: rC Clang CHANGES SINCE LAST ACTION https://

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

xazax.hun added a comment. First of all, sorry for the inactivity regarding this patch. In D58121#1437433 , @NoQ wrote: > Ok, got it! Yeah, this sounds like a valid way of supporting > non-base-region-based worklist items, i'm seeing no problems with it

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:400 + + friend class Factory; + friend class TagVisitor; NoQ wrote: > xazax.hun wrote: > > Isn't this redundant? > It isn't - i made a privat

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

xazax.hun added a comment. In D58121#1452483 , @NoQ wrote: > Mmm. I'm also pretty pinned down (it's seasonal), so i'm thinking of > temporarily reverting :( until one of us gets to fixing the accidental effect > on escaping, 'cause it just keeps biting u

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun added a comment. In D46421#1456155 , @r.stahl wrote: > Okay so I would suggest to go ahead and commit this. Rebased it succeeds > without modification. > > Still leaves the open problems with the redecls. Should I add the failing > test from ht

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun added a comment. I cannot think of other users, so I would prefer to put it in the CTU lib for now. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46421/new/ https://reviews.llvm.org/D46421 ___ cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. I have one question, once it is resolved I am fine with committing this. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:352 +return true; +} e

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:51 + // Matcher for standard smart pointers. + const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(classTemplate

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

xazax.hun added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:326 + +// We should not catch move assignment operators. +class MoveAssignOperator { While I do agree move assignment operators should not be ch

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun added a comment. Looks good, thanks. Can you commit this or do you need someone to commit it on your behalf? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46421/new/ https://reviews.llvm.org/D46421 ___ cfe

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Feel free to commit such trivial fixes without reviews. Alternatively, you could use LLVM_FALLTHROUGH, but I have no strong preference in this case.

[PATCH] D61264: Fix inconsistency in calculating DIAG_START values

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Thanks for thefix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61264/new/ https://reviews.llvm.org/D61264 _

[PATCH] D61285: [analyzer] SmartPtrModeling: Fix a null dereference.

xazax.hun accepted this revision. xazax.hun added a comment. LG! These mistakes are so easy to make. Maybe we should add nullability annotations (or use optionals) in the future? (Or just make every non-null pointer a reference and make it a convention to always check for nulls?) Repository:

[PATCH] D52957: [analyzer] Teach CallEvent about C++17 aligned new.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! I agree that it would make sense to either not have arguments that are not represented in the AST or create expressions for those implicit arguments. Repository: rC Clang htt

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:501 + auto FieldAccessM = memberExpr(hasDeclaration(equalsNode(FD))).bind("access"); + // TODO: Should we only regard asserts as guards, or maybe someth

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:519 + +if (FirstAccess->getBeginLoc() < FirstGuard->getBeginLoc()) + return true; I am not sure if this is a reliable way to chec

[PATCH] D52983: [analyzer] Support Reinitializes attribute in MisusedMovedObject check

xazax.hun created this revision. xazax.hun added reviewers: NoQ, szepet, george.karpenkov. xazax.hun added a project: clang. Herald added subscribers: Szelethus, mikhail.ramalho, a.sidorin, dkrupp, rnkovacs, baloghadamsoftware, whisperity. Support the same attribute that the tidy version of this

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

xazax.hun created this revision. xazax.hun added reviewers: NoQ, george.karpenkov, Szelethus, rnkovacs, szepet, dcoughlin, a.sidorin, MTC. xazax.hun added a project: clang. Herald added subscribers: mikhail.ramalho, dkrupp, baloghadamsoftware, whisperity. This is a first proposal to have a check

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

xazax.hun updated this revision to Diff 168664. xazax.hun added a comment. - Added the ideas from Kristof. https://reviews.llvm.org/D52984 Files: www/analyzer/checker_dev_manual.html Index: www/analyzer/checker_dev_manual.html

[PATCH] D52983: [analyzer] Support Reinitializes attribute in MisusedMovedObject check

xazax.hun added a comment. In https://reviews.llvm.org/D52983#1258466, @NoQ wrote: > Yay, these look useful. Is there also an attribute for methods that should > never be called on a 'moved-from' object? I do not know about such attribute, but once contracts are implemented and wide-spread, a

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

xazax.hun added inline comments. Comment at: www/analyzer/checker_dev_manual.html:708 +Checker Reviewer Checklist + NoQ wrote: > I think we actually need two separate checklists: > * for common bugs (careful use of non-fatal error nodes, etc. - stuff we > chec

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

xazax.hun 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 85a6dda64587a5a18482f091cbcf020fbd3ec1dd) (https://github.com/llvm-mirro

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun added a comment. Herald added a project: clang. In D46421#1375147 , @r.stahl wrote: > In D46421#1374807 , @NoQ wrote: > > > At the same time, i don't have any test cases for the actual change in > > behav

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun added a comment. Thank you for working on this! I have two concerns: - The logic when should we import the initializer of a VarDecl is duplicated. I think to have better maintainability it would be better to have only one implementation of the decision in a function and call it multi

[PATCH] D57619: [analyzer] Canonicalize variable declarations in VarRegion objects.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: cfe-commits. Looks good, nice catch. :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57619/new/ https://reviews.llvm.org/D576

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

xazax.hun added a comment. In D57230#1387834 , @NoQ wrote: > There seem to be a few regressions - weird memory leaks of inner objects in > C++ destructors. Trying to investigate/reproduce. Oh, that is unfortunate. Feel free to share a repro as soon as y

[PATCH] D57579: [analyzer][WIP] Enable subcheckers to possess checker options

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! But having a lit test that fails before and passes after would be great. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57579/new/ https://reviews.llvm.org/D57579 ___

[PATCH] D57855: [analyzer] Reimplement checker options

xazax.hun added a comment. We have `examples/analyzer-plugin`. I would prefer to add an example option to the example plugin so people do see how to do this when they are registering a checker from a plugin. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:48 +

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57922/new/ https://reviews.llvm.org/D57922 ___ cfe-commi

[PATCH] D57890: [analyzer] Fix in self assignment checker

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Just wanted to make sure I get it right. You did not add a test since it is only reproducible with an internal (non-upstreamed) checker. Since the change is trivial, I think it is o

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

xazax.hun added a comment. I think I might have a theory, but I would like to discuss it as I am not familiar with the internals bindings. My theory is the following: when we store the bindings, we store them in a map where the key is a base region. So when we try to look the bindings up wit

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

xazax.hun created this revision. xazax.hun added a reviewer: NoQ. Herald added subscribers: gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a project: clang. Follow up for a problem described in https://reviews.l

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

xazax.hun added a comment. Herald added a subscriber: jdoerfert. Experimental patch is up in https://reviews.llvm.org/D58121 Unfortunately, it is not perfect yet. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 _

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

xazax.hun updated this revision to Diff 186489. xazax.hun marked an inline comment as done. xazax.hun edited the summary of this revision. xazax.hun added a comment. Herald added a subscriber: jdoerfert. - Fix test failures. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58121/new/ https

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

xazax.hun added a comment. In D50488#1395483 , @mgrang wrote: > Reviving this now that I have some cycles to work on this. > > So I tried running this on csa-testbench projects but I didn't have much > success. I always run into a bunch of build/env relat

[PATCH] D58606: [clang-tidy] misc-string-integer-assignment: fix false positive

xazax.hun added a comment. The change looks good but it would be great to have a regression test as well. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58606/new/ https://reviews.llvm.org/D58606 ___ cfe

[PATCH] D58604: [clang-tidy] misc-string-integer-assignment: ignore toupper/tolower

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Thanks for working on this. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58604/new/ https://reviews.llvm.org/D58604 _

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Generally, it looks good to me. Though it looks like some of the cases covered in the code do not have corresponding tests (e.g.: the parenexprs). I think this approach is good in a sens

[PATCH] D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr().

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D37025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

xazax.hun added a comment. In https://reviews.llvm.org/D37023#853941, @NoQ wrote: > Thank you for the review! > > > Though it looks like some of the cases covered in the code do not have > > corresponding tests (e.g.: the parenexprs). > > These are covered by tests in `inline-defensive-checks.c:

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#854729, @dcoughlin wrote: > Except for some drive-by nits, this is a high-level review. > > I have some high-level questions about the design of the library: > > 1. **How do you intend to handle the case when there are multiple functio

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 113088. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. - Updates according to review comments. https://reviews.llvm.org/D34512 Files: include/clang/Basic/AllDiagnostics.h include/clang/Basic/CMakeLists.txt include/clang/Basic

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote: > In either case, the important scenario I think we should support is choosing > at a call site to a C function the most likely definition of the called > function, based on number and type of parameters, fr

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 113206. xazax.hun added a comment. - Added unit test to ensure the produced index format can be parsed. - Added further diagnostics. https://reviews.llvm.org/D34512 Files: include/clang/Basic/AllDiagnostics.h include/clang/Basic/CMakeLists.txt inclu

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#856821, @benlangmuir wrote: > In https://reviews.llvm.org/D34512#856301, @xazax.hun wrote: > > > In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote: > > > > > In either case, the important scenario I think we should support is

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 113740. xazax.hun marked 19 inline comments as done. xazax.hun added a comment. - Make the API capable of using custom lookup strategies for CTU - Fix review comments https://reviews.llvm.org/D34512 Files: include/clang/Basic/AllDiagnostics.h include/

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:82 +size_t Pos = Line.find(" "); +StringRef LineRef{Line}; +if (Pos > 0 && Pos != std::string::npos) { danielmarjamaki wrote: > LineRef can be const StringRef is an immu

[PATCH] D37437: [analyzer] Fix SimpleStreamChecker's output plist not containing the checker name

xazax.hun created this revision. Herald added subscribers: baloghadamsoftware, whisperity. Unfortunately, currently, the analyzer core sets the checker name after the constructor was already run. So if we set the BugType in the constructor, the output plist will not contain a checker name. Righ

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

xazax.hun updated this revision to Diff 113837. xazax.hun retitled this revision from "[analyzer] Fix SimpleStreamChecker's output plist not containing the checker name" to "[analyzer] Fix some checker's output plist not containing the checker name". xazax.hun edited the summary of this revision.

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

xazax.hun added a comment. In https://reviews.llvm.org/D37437#860311, @NoQ wrote: > Cool. Thanks! > > > In the future probably it would be better to alter the signature of the > > checkers' constructor to set the name in the constructor so it is possible > > to create the BugType eagerly. > > S

[PATCH] D37470: [analyzer] Handle ObjC messages conservatively in CallDescription

xazax.hun created this revision. Herald added subscribers: baloghadamsoftware, whisperity. The users of CallDescription no longer need to make sure that the called function is a C function. This makes the API usage easier and it conservatively will never match ObjC Messages. In the future, this

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. One nit, otherwise LGTM! Thanks for fixing this! Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:938 llvm::APSInt Multiplicand(rightI.getBitWidth(), /*

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

xazax.hun added a comment. Is this blocked on something? https://reviews.llvm.org/D32981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:35 +namespace { +// FIXME: This class is will be removed after the transition to llvm::Error. +class IndexErrorCategory : public std::error_category { dcoughlin wrote: > Is this FIX

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 114133. xazax.hun marked 8 inline comments as done. xazax.hun added a comment. - Address review comments. https://reviews.llvm.org/D34512 Files: include/clang/Basic/AllDiagnostics.h include/clang/Basic/CMakeLists.txt include/clang/Basic/Diagnostic.t

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

xazax.hun added a comment. Out of curiosity, does the false positive disappear after making the static variables const? Repository: rL LLVM https://reviews.llvm.org/D37897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

xazax.hun added a comment. I have some comments and questions but maybe you do not want to address those until Devin, NoQ, or Anna approved the general directions. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:107 +/** Recursively check if variable is changed in code. */

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. Any further reviews? What are the criteria to accept this patch? Who or how many people should accept this? https://reviews.llvm.org/D34512 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D37978: [analyzer] Fix an assertion fail in VirtualCallChecker

xazax.hun created this revision. Herald added subscribers: baloghadamsoftware, whisperity. This patch attempts to fix PR34451. The `getBestDynamicClassType` call can only handle `RecordDecls` and pointers to `RecordDecls`. The code removed all the implicit casts, this includes the array to poin

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#877032, @dcoughlin wrote: > I'm OK with this going into the repo a is (although it is light on tests!), > as long as we have an agreement that you'll be OK with iteration on both the > interface and the implementation to handle real-

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

xazax.hun added a comment. Ping. https://reviews.llvm.org/D37437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37963: [analyzer] PthreadLock: Don't track dead regions.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Maybe it worth a comment why we do not want to clean up `LockSet`? Otherwise LGTM! https://reviews.llvm.org/D37963 ___ cfe-commits mailin

[PATCH] D37812: [analyzer] PthreadLock: Escape the pointers.

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:594 +// it takes the mutex explicitly as an argument. +if (IsLibraryFunction && +std::find(ExplicitRegions.begin(), ExplicitRegions.end(), R) == Wouldn

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 116195. xazax.hun added a comment. - Unittests now creates temporary files at the correct location - Temporary files are also cleaned up when the process is terminated - Absolute paths are handled correctly by the library https://reviews.llvm.org/D34512 F

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun added a comment. In https://reviews.llvm.org/D30691#878711, @r.stahl wrote: > If either of the FullSourceLocs is a MacroID, the call > SM.getFileEntryForID(XL.getFileID()) will return a null pointer. The null > pointer will crash the program when attempting to call ->getName() on it.

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun updated this revision to Diff 116502. xazax.hun added a comment. Herald added a subscriber: baloghadamsoftware. - Fixed an issue with source locations - Updated to latest trunk https://reviews.llvm.org/D30691 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun added a comment. In https://reviews.llvm.org/D30691#878830, @r.stahl wrote: > For my purposes I replaced the return statement of the > compareCrossTUSourceLocs function with: > > return XL.getFileID() < YL.getFileID(); > > > A more correct fix would create only one unique diagnostic

[PATCH] D62885: [analyzer] Add werror flag for analyzer warnings

xazax.hun added a comment. The only problem I see with this approach is that it is an all or nothing thing at the moment. Most of the checks in CSA can have false positives and people usually do not want to fail a build due to a false positive finding. This would force the users to do two separ

[PATCH] D62556: [analyzer] NFC: CallDescription: Implement describing C library functions.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Once Aleksei's comments are resolved it is good to go. My comments are notes and not requests. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallE

[PATCH] D62557: [analyzer] Modernize CStringChecker to use CallDescriptions.

xazax.hun accepted this revision. xazax.hun added a comment. Nice one :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62557/new/ https://reviews.llvm.org/D62557 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D62441: [analyzer] NFC: Introduce a convenient CallDescriptionMap class.

xazax.hun requested changes to this revision. xazax.hun added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1088 + // descriptions (eg., the ones for C functions that just match the

[PATCH] D62619: [analyzer][Dominators] Add a control dependency tree builder + a new debug checker

xazax.hun added a comment. I did not check the algorithm as you planned changes to that. I only did a quick review of the interface which might also be rendered obsolete once you update this patch. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:191 + + CFGDom

[PATCH] D62611: [analyzer][Dominators] Add unittests

xazax.hun added a comment. Oh, I realized later that code I commented on were only moved from somewhere else. If you feel like tackling these comments feel free to do so in a separate patch so this one stays clean (no changes to moved code). Comment at: clang/unittests/Analys

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

xazax.hun requested changes to this revision. xazax.hun added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:176 + const ExplodedNode *Origin; + CFGControlDependencyTree Cont

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

xazax.hun added a comment. Generally looks good, some nits inline. Please mark comments you already solved as done. Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:73-75 static const unsigned InvalidArgIndex = UINT_MAX; /// Denotes the return vale. sta

[PATCH] D63227: [analyzer] Better timers.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Looks good :) More diagnostics are always welcome. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63227/new/ https://reviews.llvm.org/D63227 __

[PATCH] D63227: [analyzer] Better timers.

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:228 -ExprEngine::~ExprEngine() { - BR.FlushReports(); Hmm. Maybe add an assert that all the bug reports are flushed at this point? Just to make sure we never need this :

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

xazax.hun added a comment. I looked at bug path with the Optional. I did not debug into the analyzer but my intuition is the following: `DefChainEnd` is interesting. `TerminatedPaths` is the control dependency of `DefChainEnd`. And there are probably other things that are the control and/or val

[PATCH] D63227: [analyzer] Better timers.

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:228 -ExprEngine::~ExprEngine() { - BR.FlushReports(); NoQ wrote: > xazax.hun wrote: > > Hmm. Maybe add an assert that all the bug reports are flushed at this > > point?

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun marked an inline comment as done. xazax.hun added a comment. Thanks for the review! I will update the patch soon. As there is a dependency that is not accepted yet Richard (who authored the code I extended) might have some chance to take a look at this patch. Commen

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2770 +def Owner : InheritableAttr { + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; aaron.ballman wrote: > xazax.hun wrote: > > aaron.ballman wr

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

xazax.hun added a comment. I have one small question otherwise looks good. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1457 + if (TKind == TrackingKind::ConditionTracking && + StoreSite->getStackFrame() == OriginSFC) +return nullptr;

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! Since we allow new kinds of SVals to be tracked it would be great to test this first on a larger corpus of projects just to see if there is a crash (due to an unhandled SVal type). CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81 + +bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, + const Expr *E, BugReport &report, Do we need thi

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

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1052 +// to return that every time. +if (N->getCFG().isLinear()) + WouldEventBeMeaningless = true; NoQ wrote: > This time i'd rather go for

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun updated this revision to Diff 211127. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Address the rest of the review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64256/new/ https://reviews.llvm.org/D64256 Files: clang/include/clang/Basic/

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun updated this revision to Diff 211142. xazax.hun added a comment. - Fix a TODO and add some more tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64256/new/ https://reviews.llvm.org/D64256 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaInit.

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun updated this revision to Diff 211150. xazax.hun added a comment. - Fix a false positive from previous change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64256/new/ https://reviews.llvm.org/D64256 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/Se

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

xazax.hun created this revision. xazax.hun added reviewers: gribozavr, rsmith, mgehre. xazax.hun added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, Szelethus, dkrupp, rnkovacs. This patch extends the warnings for additional cases: 1. When the temporary is the res

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/inner-pointer.cpp:2 // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \ -// RUN: %s -analyzer-output=text -verify I had to disable

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6564 +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee, + const CXXMemberCallExpr *MCE) { + if (auto *

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

xazax.hun created this revision. xazax.hun added reviewers: gribozavr, rsmith, mgehre. xazax.hun added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, Szelethus, dkrupp, rnkovacs. xazax.hun added a parent revision: D65120: More warnings regarding gsl::Pointer and gsl

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

xazax.hun added a comment. I have run this successfully on ~192 open source projects. The results so far: 1. https://github.com/ANTsX/ANTs/blob/master/Examples/sccan.cxx#L2899 /home/tgahor/data/projects/ANTs/Examples/sccan.cxx:2899:34: warning: object backing the pointer will be destroyed

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun updated this revision to Diff 211580. xazax.hun added a comment. - Fix a false positive case found by running over ~200 open source projects CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64256/new/ https://reviews.llvm.org/D64256 Files: clang/include/clang/Basic/DiagnosticS

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

xazax.hun updated this revision to Diff 211581. xazax.hun added a comment. - Remove unused parameter. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65120/new/ https://reviews.llvm.org/D65120 Files: clang/lib/Sema/SemaInit.cpp clang/test/Analysis/inner-pointer.cpp clang/test/Sema/

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

xazax.hun updated this revision to Diff 211583. xazax.hun added a comment. - Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65127/new/ https://reviews.llvm.org/D65127 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/warn-lifetime-analysis-nocfg.cpp Index: clang/test/Sema/w

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun updated this revision to Diff 212204. xazax.hun marked an inline comment as done. xazax.hun added a comment. - A small refactoring based on an observation from a review comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64256/new/ https://reviews.llvm.org/D64256 Files:

<    1   2   3   4   5   6   7   8   9   10   >