[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:270 + // needed. + BoolValue &ComparisonValue = MakeValue(Env, *HasValueVal); + auto *ComparisonExprLoc = ymandel wrote: > xazax.hun wrote: >

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:307 + [](Environment &Env, BoolValue &HasValueVal) -> BoolValue & { +// We can't reason about general vlaues, so we encode the constraint on +

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:270 + // needed. + BoolValue &ComparisonValue = MakeValue(Env, *HasValueVal); + auto *ComparisonExprLoc = xazax.hun wrote: > ymandel wrote: >

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:279 + cast_or_null(Env.getValue(*ComparisonExprLoc))) { +Env.setValue(*ComparisonExprLoc, + Env.makeAnd(*CurrentValue, Compar

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:295 +// atom. +BoolValue &OptionalHoldsEmptyString = Env.makeAtomicBoolValue(); +return Env.makeOr(Env.makeAnd(HasValueVal, OptionalHolds

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Wow. This did take some iterations and I feel like I just added to the confusion at some point :D But the latest iteration looks much simpler and I'm confident it is right this time. Tha

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:185 + // base classes, because they are not visible in derived classes. + getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true, +

[PATCH] D122830: [clang][dataflow] Add support for (built-in) (in)equality operators

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:56 +Env.getValue(*RHSNorm, SkipPast::Reference))) + return Env.makeIff(*LHSValue, *RHSVal

[PATCH] D122273: [clang][dataflow] Fix handling of base-class fields

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:185 + // base classes, because they are not visible in derived classes. + getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateF

[PATCH] D122838: [clang][dataflow] Add support for correlation of boolean (tracked) values

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:79 +for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) { + Expr1 = &Env1.makeAnd(*Expr1, *Constraint); +} Hmm, interesting. I think we

[PATCH] D122838: [clang][dataflow] Add support for correlation of boolean (tracked) values

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:79 +for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) { + Expr1 = &Env1.makeAnd(*Expr1, *Constraint); +} ---

[PATCH] D122908: [clang][dataflow] Add support for clang's `__builtin_expect`.

xazax.hun accepted this revision. xazax.hun added a comment. Nice! Speaking of builtins, it might be great to add tests for `__builtin_unreachable`, `__builtin_trap`, `__builtin_debugtrap`. The CFG might already have the right shape so we might not need to add any code to support them. But it

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:513 + const RecordDecl *RD = BaseTy->getDecl(); + if (RD->getIdentifier() == nullptr || RD->getName() != "Message") +return false; Not sure how often is th

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:513 + const RecordDecl *RD = BaseTy->getDecl(); + if (RD->getIdentifier() == nullptr || RD->getName() != "Message") +return false; ymandel wrote: > xazax.h

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:513 + const RecordDecl *RD = BaseTy->getDecl(); + if (RD->getIdentifier() == nullptr || RD->getName() != "Message") +return false; xazax.hun wrote: > ymand

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

xazax.hun added reviewers: NoQ, steakhal, Szelethus, martong, xazax.hun. xazax.hun added a comment. Herald added a subscriber: rnkovacs. Adding some reviewers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123155/new/ https://reviews.llvm.org/D12315

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. The changes look good to me but please wait at least one more reviewer before committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. In D130306#3679294 , @samestep wrote: > @xazax.hun Do you have anything else you'd like addressed/changed (e

[PATCH] D130593: [clang][dataflow] Separate context by frame

xazax.hun added a comment. While separate call strings are properly isolated from each other, repeated analysis of the same call is not:| while (...) { foo(...); } In the above code snippet, we will end up analyzing foo with leftover state from the previous iteration. The analysis ca

[PATCH] D130726: [clang][dataflow] Handle multiple context-sensitive calls to the same function

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. I'd love to see some comments in the source code to make sure one will not sue these facilities for other purposes that this was not designed to do. Alternatively, `ContextSensitive` opt

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:38 + + // DEPRECATED. Use overload above. static llvm::Expected build(const Decl *D, Stmt *S, I think LLVM can use the `[[deprecated]]` attribute. =

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

xazax.hun added a comment. The concept of fully qualified name is somewhat ambiguous for me. Do we expect the user to specify inline namespaces as well? I'd love to see some test cases and comments that clarify this. Comment at: clang/lib/Analysis/FlowSensitive/ControlFlowCon

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

xazax.hun added a subscriber: gribozavr2. xazax.hun added a comment. In D131280#3706915 , @ymandel wrote: > Sure. This is probably worth some discussion. Fully qualified names, however > we define them, will not be enough, since they don't cover overload

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

xazax.hun added a comment. In D131280#3709781 , @ymandel wrote: > Thanks. That looks good, but I'm concerned that it only counts the arguments > and doesn't look at their types. I'd imagine this will be a limitation down > the line when we want to deal

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

xazax.hun added a comment. In D131280#3710080 , @ymandel wrote: > I think we should try to reuse for the reasons you mention. We can evolve it > with more features as needed. I'm kind of allergic to `const char *` so I > propose that I first send a patc

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added a comment. I feel like this is a repeated pattern. The CSA solved a very similar issue by introducing the CallEvent class hierarchy. I also remember seeing many disparate code snippets littered throughout the clang codebase that tries to deal with the problem of not having facil

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:248-258 + for (unsigned ArgIndex = 0; ArgIndex < NumArgs; ++ParamIt, ++ArgIndex) { assert(ParamIt != FuncDecl->param_end()); -const Expr *Arg = *ArgIt; -auto *Arg

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:248-258 + for (unsigned ArgIndex = 0; ArgIndex < NumArgs; ++ParamIt, ++ArgIndex) { assert(ParamIt != FuncDecl->param_end()); -const Expr *Arg = *ArgIt; -auto *Arg

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:219-245 // FIXME: Support references here. - Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); + ReturnLoc = getStorageLocation(*Call, SkipPast::Reference)

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun accepted this revision. xazax.hun added a comment. Nice! We went from branches in the loop to branches before the loop first. And now the branches are at compile time during overload resolution. I love it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

xazax.hun requested changes to this revision. xazax.hun added a comment. This revision now requires changes to proceed. Commented some nits, but overall looks good to me. However, could you include some tests? We usually do not commit any changes without tests unless it is really hard to create

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

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

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:291 // we can try this function - if (Call.getNumArgs() == 2 && - Call.getDecl()->getDeclContext()->isStdNamespace()) -if (smartptr::isStdSmartPtr(Call.getArgExpr(0))

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

xazax.hun added a comment. In D120992#3368118 , @NoQ wrote: > I guess there's the usual direction that I occasionally suggest: develop a > way to verify that all possible paths were explored during symbolic execution > (`CoreEngine::hasWorkRemaining()`

[PATCH] D121197: [clang][dataflow] Add analysis that detects unsafe accesses to optionals

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:115 + // optional::has_value + .CaseOf(isOptionalMemberCallWithName("has_value"), + transferOptionalHasV

[PATCH] D121378: [clang][dataflow] Model the behavior of various optional members

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:64 static BoolValue *getHasValue(Value *Val) { - if (auto *OptionalVal =

[PATCH] D121455: [clang][dataflow] Add support for nested composite bool expressions

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:516 +// assigned to it. +Visit(&SubExpr); +if (auto *Val = dyn_cast_or_null( Could you elaborate on when would this happen? I'd expect the traversal to always

[PATCH] D121455: [clang][dataflow] Add support for nested composite bool expressions

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:516 +// assigned to it. +Visit(&SubExpr); +if (auto *Val = dyn_cast_or_null( sgatev wrote: > xazax.hun wrote: > > Could you elaborate on when would this happen?

[PATCH] D121455: [clang][dataflow] Add support for nested composite bool expressions

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:516 +// assigned to it. +Visit(&SubExpr); +if (auto *Val = dyn_cast_or_null( sgatev wrote: > xazax.hun wrote: > > sgatev wrote

[PATCH] D121602: [clang][dataflow] Model the behavior of non-standard optional constructors

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:55 + return hasType( + recordDecl(anyOf(hasName("std::nullopt_t"), hasName("absl::nullopt_t"), + hasName("base::nullopt_t"; -

[PATCH] D121602: [clang][dataflow] Model the behavior of non-standard optional constructors

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:146 + std::vector> + Actions; Nit: looks like we need to repeat the action type. Should we restore the using above? Repository: rG LLVM Github Monorep

[PATCH] D121602: [clang][dataflow] Model the behavior of non-standard optional constructors

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:118 +/// function will be 2. +int countOptionalWrappers(const ASTContext &ASTCtx, QualType Type) { + if (!IsOptionalType(Type)) Nit: some of th

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

xazax.hun accepted this revision. xazax.hun added a comment. The change itself looks good. But out of curiosity, could you give me an example when we do not want to use the builtin transfer functions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D121694: [clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

xazax.hun added a comment. In D121694#3382587 , @ymandel wrote: > In D121694#3382473 , @xazax.hun > wrote: > >> The change itself looks good. But out of curiosity, could you give me an >> example when we do not

[PATCH] D121796: [clang][dataflow] Add an API for dataflow "models" -- reusable analysis components.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:150-151 +///should relate to each other -- that is, how they should compose. Open +///questions include: Do we want to enable composition of models that have +///

[PATCH] D121796: [clang][dataflow] Add an API for dataflow "models" -- reusable analysis components.

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121796/new/ https://reviews.llvm.org/D121796 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D121797: [clang][dataflow] Add modeling of Chromium's CHECK functionality

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp:122 + void transfer(const Stmt *S, NoopLattice &, Environment &Env) { +M.transfe

[PATCH] D121797: [clang][dataflow] Add modeling of Chromium's CHECK functionality

xazax.hun added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp:122 + void transfer(const Stmt *S, NoopLattice &, Environment &Env) { +M.transfer(S, Env); + } ymandel wrote: > xazax.hun wrote: > > I wonder whe

[PATCH] D121863: [clang][dataflow] Model the behavior of non-standard optional assignment

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:81 +auto isOptionalValueOrConversionAssignment() { + return cxxOperatorCallExpr( While really like the convenience matchers will give us buil

[PATCH] D121797: [clang][dataflow] Add modeling of Chromium's CHECK functionality

xazax.hun added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp:122 + void transfer(const Stmt *S, NoopLattice &, Environment &Env) { +M.transfer(S, Env); + } ymandel wrote: > xazax.hun wrote: > > ymandel wrot

[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

xazax.hun added a comment. Are smart pointers special? I would expect to see similar problems with containers (or even a nested optional). I wonder whether an allowlist instead of a denylist approach is better here. E.g., instead of disabling the modeling for smart pointers, we could enable it

[PATCH] D122121: [clang][dataflow] Add action caching support to MatchSwitch

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Did you have a chance whether this makes a difference in real world scenarios? I'm mostly curious because I do not have a good mental model of how the matchers are implemented, specifically what optimizations are in place, so

[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

xazax.hun added a comment. In D122143#3396695 , @ymandel wrote: > So, do you mean to add a FIXME to move to allowlist, or do you mean to hold > off until we've switched? I have a short-term interest in getting this > through for a particular usecase, bu

[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:64 + callee(cxxMethodDecl(ofClass(optionalClass(, + unless(hasArgument(0, *Ignorable))); + retur

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

xazax.hun added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6387 +additional information specific to the annotation category. The optional +arguments must be constant expressions of arbitrary type. + Do we want to mention that it is not act

[PATCH] D123586: [clang][dataflow] Weaken abstract comparison to enable loop termination.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Yeah, this is a hard problem in general. This looks like a sensible workaround for the short term, but I'm looking forward to a better solution. I'm a bit worried that the memory model w

[PATCH] D123586: [clang][dataflow] Weaken abstract comparison to enable loop termination.

xazax.hun added a comment. In D123586#3449256 , @ymandel wrote: > In D123586#3446956 , @xazax.hun > wrote: > >> Yeah, this is a hard problem in general. This looks like a sensible >> workaround for the short ter

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

xazax.hun added a comment. > Make CTU a two phase analysis At this point maybe it will be a three phase, as we might dump the ASTs/create index in phase 1 :) > During this phase, if we find a foreign function (that could be inlined from > another TU) then we don’t inline that immediately, we r

[PATCH] D123961: [clang][dataflow] Do not crash on missing `Value` for struct-typed variable init.

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:184-186 - // FIXME: The initializer expression must always be assigned a value. - // Replace thi

[PATCH] D123858: [clang][dataflow] Ensure well-formed flow conditions.

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:110 +// FIXME: The flow condition must be an r-value, so `SkipPast::None` should +

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:69 /// Attempts to merge distinct values `Val1` and `Val1` in `Env1` and `Env2`, /// respec

[PATCH] D119953: [clang][dataflow] Add transfer functions for logical and, or, not.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:28 + + /// Returns the environment of the basic block that contains `S` or nullptr if + /// there isn't one. Depending on how willing sensitive we are to memory

[PATCH] D119953: [clang][dataflow] Add transfer functions for logical and, or, not.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:28 + + /// Returns the environment of the basic block that contains `S` or nullptr if + /// there isn't one. xazax.hun wrote: > Depending on how willing sensitiv

[PATCH] D124395: [clang][dataflow] Optimize flow condition representation

xazax.hun added a comment. Nice! Did you do some measurements? Does this improve the performance or decrease the memory consumption? Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:160 + void addFlowConditionConstraint(AtomicBoolValue &Token,

[PATCH] D120495: [clang][dataflow] Add transfer functions for structured bindings

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:80 +/// +/// FIXME: Consider adding support for structured bindings to the CFG builder. +class DecompositionVisitor : public ConstStmtVisitor { sgatev wrote: > xazax.hun wr

[PATCH] D124807: [clang][dataflow] Avoid assert for invalid cast to BoolValue

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:78 +/// and integers in the framework. +static const Expr *ignoreParenImpCastsExceptToBool(const Expr *E) { + const Expr *LastE = nullptr; li.zhe.hua wro

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:78 +/// and integers in the framework. +static const Expr *ignoreParenImpCastsExceptToBo

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

xazax.hun added a comment. Overall looks good to me. I am curious what will the strategy be to properly support construction. Do you plan to introduce a customization point to `Env.createValue` to give checks/models a way to set properties up? Or do you have something else in mind? =

[PATCH] D124943: [clang][dataflow] Add flowConditionIsTautology function

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. The code looks good to me too. I was also wondering what sort of check will need this info. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

xazax.hun added a comment. Thanks for the clarifications! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124932/new/ https://reviews.llvm.org/D124932 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added a comment. Most checks in Clang Tidy will run relatively quickly as they usually can do most/all of their work in a single traversal. I wonder whether flow sensitive checks will prove to be a bit slower. I think adding slower checks to Tidy is fine, but it would be nice to prope

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:38 + + auto HasOptionalCallDescendant = hasDescendant(callExpr(callee(cxxMethodDecl( + ofClass(anyOf(hasName("std::optional"), hasName("absl::optional"), ---

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:84 + if (!BlockToOutputState || + BlockToOutputState->size() <= Context->getCFG().getExit().getBlockID()) +return; Could the size of the

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:84 + if (!BlockToOutputState || + BlockToOutputState->size() <= Context->getCFG().getExit().getBlockID()) +return; xazax.hun wrote: > Co

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Overall this looks good to me. However, I think this might not use the full potential of the check itself. With the information that the dataflow framework have it could distinguish betw

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst:29 + +Checking if a value exists, then accessing the value + ymandel wrote: > xazax.h

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:84 + if (!BlockToOutputState || + BlockToOutputState->size() <= Context->getCFG().getExit().getBlockID()) +return; ymandel wrote: > xaza

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:84 + if (!BlockToOutputState || + BlockToOutputState->size() <= Context->getCFG().getExit().getBlockID()) +return; xazax.hun wrote: > ym

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun accepted this revision. xazax.hun added a comment. It looks like all of my comments are resolved now, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121120/new/ https://reviews.llvm.org/D121120

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:84 + if (!BlockToOutputState || + BlockToOutputState->size() <= Context->getCFG().getExit().getBlockID()) +return; ymandel wrote: > xaza

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

xazax.hun added a comment. In D121120#3496597 , @ymandel wrote: > Just realized we left this open. How about a disclaimer at the top of the doc > (.rst) file noting the potential cost? Anyone using clang-tidy should be > explicitly configuring which ch

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

xazax.hun added inline comments. Comment at: clang/include/clang/AST/ASTImporterSharedState.h:43 + /// Set of the newly created declarations. + llvm::DenseSet NewDecls; + ASTImporter already has something like `ImportedFromDecls`. Is that not sufficient to che

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116 + const bool Foreign = false; // From CTU. + I feel that we use different terms for the imported declarations. Sometimes we call them `new`, so

[PATCH] D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order

xazax.hun added a comment. This approach fixes the worklist for the second phase. Would it be possible to create a wrapper that reverses the order of any worklist instead of committing to one and hardcode that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Thanks, this looks good to me! Comment at: clang/include/clang/AST/ASTImporterSharedState.h:43 + /// Set of the newly created declarations. + llvm::DenseSet NewDecls;

[PATCH] D129547: [clang][dataflow] Generate readable form of boolean values for debugging purposes.

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp:28 + +class DebugStringGenerator { +public: This class could be in anonymous namespac

[PATCH] D129548: [clang][dataflow] Generate readable form of input and output of satisfiability checking for debugging purposes.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Some nits inline, but looks good to me. Comment at: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp:119-133 +std::vector> LinesData; +for (auto &AtomAssignmen

[PATCH] D129548: [clang][dataflow] Generate readable form of input and output of satisfiability checking for debugging purposes.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp:230 + +Unsatisfiable. + wyt wrote: > @xazax.hun > > I don't see a test case for `Unsatisfiable` constraints. >

[PATCH] D130305: [clang][dataflow] Refactor ApplyBuiltinTransfer field out into DataflowAnalysisOptions struct

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h:38 + // analysis). + bool ApplyBuiltinTransfer; +}; I think it might be better to have a default value. Repository: rG LLVM Github Monorepo

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

xazax.hun added a comment. There are many ways to introduce context sensitivity into the framework, this patch seems to take the "inline substitution" approach, the same approach the Clang Static Analyzer is taking. While this approach is relatively easy to implement and has great precision, it

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

xazax.hun added a comment. In D130306#3671852 , @samestep wrote: > In D130306#3670259 , @xazax.hun > wrote: > >> > > Yes, we considered a summary-based approach, but we decided not to use it > because (as you m

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

xazax.hun added a comment. Thanks! Knowing the context, I am much happier with the direction overall. Is the plan to analyze a mock of std::optional instead of the actual code in the STL? How will that mock be shipped? Would that be embedded in the binary? In D130306#3676475

[PATCH] D130519: [clang][dataflow] Add explicit "AST" nodes for implications and iff

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:332 + + if (LeftSubVar == RightSubVar) { +// `X <=> (A <=> A)` is equvalent to `X` which is already in I wonder why this simplification is done he

[PATCH] D130519: [clang][dataflow] Add explicit "AST" nodes for implications and iff

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:332 + + if (LeftSubVar == RightSubVar) { +// `X <=> (A <=> A)` is equvalent to `X` which is already in xazax.hun wrote: > I wonder why this simpli

[PATCH] D130519: [clang][dataflow] Add explicit "AST" nodes for implications and iff

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:332 + + if (LeftSubVar == RightSubVar) { +// `X <=> (A <=> A)` is equvalent to `X` which is already in gribozavr2 wrote: > xazax.hun wrote: > > xa

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446 +} +const bool BState = State->contains(D); +if (!BState) { // This is the first time we see this foreign function. martong wrote: > xazax.hun w

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446 +} +const bool BState = State->contains(D); +if (!BState) { // This is the first time we see this foreign function. xazax.hun wrote: > martong w

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116 + const bool Foreign = false; // From CTU. + martong wrote: > martong wrote: > > xazax.hun wrote: > > > I feel that we use different terms for t

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Overall looks good to me. I wish some parts would be simpler but it looks like sometimes it is not easy to extend the current code and we might need to do some refactoring at some point.

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