[PATCH] D55131: [CTU] Add more lit tests and better error handling

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/D55131/new/ https://reviews.llvm.org/D55131 ___

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

xazax.hun updated this revision to Diff 176403. xazax.hun marked 4 inline comments as done. xazax.hun added a comment. - Addressed further comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52984/new/ https://reviews.llvm.org/D52984 Files: www/analyzer/checker_dev_manual.html

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

xazax.hun added a comment. Hm. I wonder if it would also make sense to model e.g. the get method to return nullptr for moved from smart ptrs. This could help null dereference checker and also aid false path prunning. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii updated this revision to Diff 176732. hgabii marked an inline comment as done. hgabii edited the summary of this revision. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/ https://reviews.llvm.org/D42682 Files:

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii marked an inline comment as done. hgabii added a comment. I moved to bugprone module and renamed to bugprone-io-functions. I added as a reference for cert-fio34-c. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii updated this revision to Diff 176736. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/ https://reviews.llvm.org/D42682 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt

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

xazax.hun accepted this revision. xazax.hun added a reviewer: xazax.hun. xazax.hun added a comment. Some minor comment inline. Otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:879 +// If this token is the current macro's argument, we should

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

hgabii marked 9 inline comments as done. hgabii added a comment. I resolved all comments. Comment at: docs/ReleaseNotes.rst:60 +- New :doc:`misc-incorrect-pointer-cast + ` check Eugene.Zelenko wrote: > Will be good idea to rebase from trunk and use

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Overall looks good to me, some minor comments inline. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:231 #define ANALYZER_OPTION(TYPE, NAME, CMDFLAG,

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Some nits inline. Otherwise looks good to me. Comment at: include/clang/CrossTU/CrossTranslationUnit.h:32 class FunctionDecl; +class VarDecl; class NamedDecl;

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

xazax.hun marked 2 inline comments as done. xazax.hun added a comment. In D57230#1372275 , @NoQ wrote: > Could you share reproducible examples for these, probably in the form of > FIXME tests? Given that they are "regressions", they are easy to creduce

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

xazax.hun updated this revision to Diff 183702. xazax.hun added a comment. - Added some tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 Files: lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/call-invalidation.cpp

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

xazax.hun added a comment. In D57230#1372523 , @NoQ wrote: > In D57230#1372488 , @xazax.hun wrote: > > > In D57230#1372275 , @NoQ wrote: > > > > > > > > > > > Do you have

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

xazax.hun added a comment. I tried to creduce one file where the result differed and this is the result: typedef struct { int a; int b } c; d; e(c *f) { d < f->a; c g; h(); e(); } I think this the core idea is quite straightforward but this example is a bit

[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

[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

[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] 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 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

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! I think we should commit this as is for now but maybe adding a TODO comment to summarize the problem would be nice. Maybe we could have an isSameDialect or similar method within

[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 ___

[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] 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

[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] 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

[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

[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/

[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

[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] 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

[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

[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

[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 ___

[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

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

xazax.hun created this revision. xazax.hun added reviewers: dergachev.a, george.karpenkov, Szelethus. xazax.hun added a project: clang. Herald added subscribers: gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. This is a patch for the

[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

[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 > >

[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/

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

xazax.hun added a comment. Thanks for all the reviews. Do you have any preference about the spelling of the annotation mentioned in the description? There were two ideas so far: `uses_offsetof`, `may_use_offsetof` While I like those, I wonder if it is a good idea to have `offsetof` in the

[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; +}

[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( +

[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

[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

[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 + +

[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:

[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

[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

[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:

[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.

[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

[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] 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] D62883: [analyzer] Track terminator conditions on which a tracked expressions depends

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Artem had some comments that are not marked as done, but LGTM! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:157 + /// Conditions

[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] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

xazax.hun added a comment. In D62883#1552870 , @Szelethus wrote: > - Now using `CFGBlock::getTerminatorConditionExpr()` > - Uniqueing already tracked conditions as an (`Expr`, `ExplodedNode`) pair > instead of on `Expr` I would be surprised to see the

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

xazax.hun added a comment. Mostly looks good, I have two slightly related nits. Comment at: include/clang/CrossTU/CrossTranslationUnit.h:127 + StringRef IndexName, bool DisplayCTUProgress, + unsigned CTULoadThreshold);

[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] D64256: Teach some warnings to respect gsd::Pointer and gsl::Owner attributes

xazax.hun created this revision. xazax.hun added reviewers: mgehre, rsmith, gribozavr. Herald added subscribers: cfe-commits, Charusso, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a project: clang. This patch extends some existing warnings to utilize the knowledge about the

[PATCH] D63538: [analyzer][CFG] Return the correct terminator condition

xazax.hun added a comment. This change will be really useful for the lifetime analysis as well! Thanks! I have one concern though. The analyzer is using linerarized (or threaded) CFGs with every subexpression being a separate entry in the basic blocks. Will your change give sensible answers

[PATCH] D63878: [CTU] Add missing statistics

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

[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

[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.

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

xazax.hun marked 6 inline comments as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:7076 +auto Prefix = llvm::makeArrayRef(Path).drop_back(); + if (pathInitializeLifetimePointer(Prefix)) +IsLifetimePtrInitWithTempOwner =

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

xazax.hun updated this revision to Diff 208709. xazax.hun added a comment. - Address review comments. 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.cpp

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

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Looks good to me, some nits inline. Comment at: clang/unittests/Analysis/CFGBuildResult.h:1 +//===- unittests/Analysis/CFGTest.cpp - CFG tests

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

xazax.hun added a comment. Some nits inline, note that this was just a partial review. Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:14 + +#include "clang/Driver/DriverDiagnostic.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"

[PATCH] D63538: [CFG] Add a new function to get the proper condition of a CFGBlock

xazax.hun added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5619-5625 + // If the terminator is a temporary dtor or a virtual base, etc, we can't + // retrieve a meaningful condition, bail out. + if (rbegin()->getKind() != CFGElement::Kind::Statement) +return

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added a comment. In D63954#1564448 , @gribozavr wrote: > > We explicitly allow to add an annotation after the definition of the class > > to allow adding annotations to external source from by the user > > Asking users to forward-declare

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4553 + // we always add (and check) the attribute to the cannonical decl. + D = D->getCanonicalDecl(); + if(AL.getKind() == ParsedAttr::AT_Owner) { aaron.ballman wrote: > xazax.hun

[PATCH] D63968: [analyzer] Fix target region invalidation when returning into a ctor initializer.

xazax.hun accepted this revision. xazax.hun added a comment. LG! Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63968/new/ https://reviews.llvm.org/D63968 ___ cfe-commits mailing list

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

xazax.hun updated this revision to Diff 208441. xazax.hun added a comment. - Fix a typo. 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.cpp

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

xazax.hun updated this revision to Diff 208439. xazax.hun marked 12 inline comments as done. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64256/new/ https://reviews.llvm.org/D64256 Files:

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

xazax.hun added inline comments. Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:22 + int *release2(); + int *c_str() const; +}; gribozavr wrote: > This method is confusing -- is it a name that the warning is supposed to know > about? Not yet,

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

xazax.hun marked 5 inline comments as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6510 LifetimeBoundCall, +LifetimePointerInit, +LifetimeTempOwner gribozavr wrote: > What is this name supposed to mean?

[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] 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 ___

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added a comment. In D63954#1565109 , @gribozavr wrote: > > I agree. In a follow-up patch we will add the attributes to STL types > > during parsing. Do you think it is OK to always add the attributes or > > should we only do it if a flag, e.g.

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added a comment. I actually think we should hard code STL (and maybe some other popular) types into the compiler. I.e.: if an STL type is unannotated and the warnings are turned on, we could look up the default annotations from a table and append them during parsing. This could be

[PATCH] D63956: [analyzer] NonnullGlobalConstants: Don't be confused with a _Nonnull attribute.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LG! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63956/new/ https://reviews.llvm.org/D63956

[PATCH] D66152: Fix false negatives of statement local lifetime analysis for some STL implementation

xazax.hun added a comment. In D66152#1627572 , @Szelethus wrote: > Apologies for going off-topic, but would it make sense to start lifetime > related patches with the tag [LifetimeAnalysis] or similar (like [analyzer] > for static analyzer patches)? I

[PATCH] D66152: Fix false negatives of statement local lifetime analysis for some STL implementation

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:7130 -bool IsGslPtrInitWithGslTempOwner = -IsTempGslOwner && pathOnlyInitializesGslPointer(Path); +bool IsGslPtrInitWithGslTempOwner = false; +

[PATCH] D66152: Fix false negatives of statement local lifetime analysis for some STL implementation

xazax.hun created this revision. xazax.hun added reviewers: gribozavr, mgehre. Herald added subscribers: Charusso, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a project: clang. xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at:

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

xazax.hun added inline comments. Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172 T *(); + T (); +}; mgehre wrote: > xazax.hun wrote: > > I actually was a bit lazy when I added these tests. Both `value` and > > `operator*` is overloaded on

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6583 .Cases("end", "rend", "cend", "crend", true) -.Cases("c_str", "data", "get", true) +.Cases("c_str", "data", "get", "value", true) // Map and set types.

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LG! But let's wait for Dmitri :) Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172 T *(); + T (); +};

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

xazax.hun added a comment. The direction looks good to me. I wonder if it is worth to add some warnings somewhere that it is really tricky to come up with reasonable fixits for most path sensitive checks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/

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

xazax.hun added a comment. I like the change. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:630 +(const RegionBindings::TreeTy *)((uintptr_t)store & ~(uintptr_t)1), +RBFactory.getTreeFactory(), (bool)((uintptr_t)store & (uintptr_t)1)); }

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

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! I also do not see much value in the old code at this point. I would expect PathDiagnosticConsumers to be independent of the interesting symbols/regions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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/D65349/new/ https://reviews.llvm.org/D65349 ___

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

xazax.hun added a comment. Just wondering, did you check if we actually need shared ownership for this type? If not, do not waste your time checking for now :) Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:244 +

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

xazax.hun accepted this revision. xazax.hun added a comment. Looks good, some nits inline. I agree with Artem, we should consider omitting the trimming and document how to get something similar if desired for debugging. Just a random idea: maybe the original purpose of the trimming was to be

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

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/CFG.h:860 + /// child CFG blocks in a depth-first manner and see if all paths eventually + /// end up in an immediate sink block. + bool isInevitablySinking() const; Is it obvious what

[PATCH] D64991: [analyzer][WIP] Implement a primitive reaching definitions analysis

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ReachingDefinitions.h:30 + }; + const VarDecl *Var; + CFGBlock::ConstCFGElementRef E; In the future we might also want to reason about `FieldDecl`s. Comment

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

xazax.hun accepted this revision. xazax.hun added a comment. More tests are always welcome :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65290/new/ https://reviews.llvm.org/D65290 ___ cfe-commits

[PATCH] D66303: [LifetimeAnalysis] Add support for free functions

xazax.hun created this revision. xazax.hun added reviewers: mgehre, gribozavr. xazax.hun added a project: clang. Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs. This patch adds support to the idiom when people using the free version of `begin`, `end` and the like

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

xazax.hun added a comment. In D64256#1626279 , @leonardchan wrote: > In D64256#1626025 , @xazax.hun wrote: > > > In D64256#1625998 , @leonardchan > > wrote: > > > > > Hi. I

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