[PATCH] D148554: [dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:15-16 + +// The Arena owns the objects that model data within an analysis. +// Currently this is mo

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:43 +namespace { + mboehme wrote: > There were a number of pre-exist

[PATCH] D149640: [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

gribozavr2 added a comment. Is it testable? For example, can we show that we don't compute the program state for the program point "p" here: void f() { while (true) { } /*p*/ } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149640/n

[PATCH] D149640: [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2641 TEST(TransferTest, VarDeclInDoWhile) { std::string Code = R"( Could you update the test name to reflect what the test is checking now? (the comment t

[PATCH] D152732: [clang][dataflow] Support limits on the SAT solver to force timeouts.

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h:37 + // measurements like CPU cycles or time to ensure deterministic results. + std::int64_t MaxIterations = std::numeric_limits::max(); + Consider

[PATCH] D152813: [clang][dataflow] Create `Value`s for integer literals.

gribozavr2 added a comment. In D152813#4420399 , @mboehme wrote: > It looks to me as if the values we're now newly producing for integer > literals are causing non-convergence of the analysis on the for-loop. For integer literals specifically, we should

[PATCH] D152813: [clang][dataflow] Create `Value`s for integer literals.

gribozavr2 added a comment. In D152813#4420448 , @mboehme wrote: > We would expect analysis of this code to converge too -- right? Yes - but we might need universal top values for that? maybe? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D152813: [clang][dataflow] Create `Value`s for integer literals.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:89 + /// `Value`. These literals are the same every time. + IntegerValue &makeIntLiteral(llvm::APInt Value); + Should we be takin

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG82d50fef9b7c: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice… (authored by merrymeerkat, committed by gribozavr).

[PATCH] D140037: [clang][dataflow] Remove old diagnoser API

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4ca1ae580b0c: [clang][dataflow] Remove old diagnoser API (authored by merrymeerkat, committed by gribozavr). Repository: rG LLVM Github Monorepo

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

gribozavr2 added inline comments. Comment at: clang/include/clang/AST/Type.h:2550 /// be lost by canonicalization and desugaring. - Optional getNullability(const ASTContext &context) const; + Optional getNullability() const; Could you add a temporary depre

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0da4cecfb6ad: [clang][dataflow] Remove unused argument in getNullability (authored by merrymeerkat, committed by gribozavr). Repository: rG LLVM G

[PATCH] D117304: [clang][dataflow] Remove TestingSupport's dependency on gtest

gribozavr2 added a comment. > Why does it matter what version outside users are using -- these are clang > unit-tests, not a public API, right? clang/unittests/Analysis/FlowSensitive/TestingSupport.h is a public API for writing tests for dataflow analyses. Repository: rG LLVM Github Monorep

[PATCH] D114231: [clang][docs][dataflow] Added an introduction to dataflow analysis

gribozavr2 added a comment. > Did you add somewhere what dependency we need in order to build the > documentation now as mine is having problems with "recommonmark" I didn't change the documentation because recommonmark is not a new dependency for llvm-project, llvm/docs was depending on it bef

[PATCH] D116190: Comment parsing: Don't recognize commands in single-line double quotation

gribozavr2 added inline comments. Comment at: clang/lib/AST/CommentLexer.cpp:278 + +again: + size_t End = I'd suggest refactoring to a `while (true)` if you don't mind. Comment at: clang/lib/AST/CommentLexer.cpp:290 +if (End != StringRef::

[PATCH] D116190: Comment parsing: Don't recognize commands in single-line double quotation

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/CommentLexer.cpp:278 + +again: + size_t End = aaronpuchert wrote: > gribozavr2 wrote: > > I'd suggest refactoring to a

[PATCH] D116186: Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC)

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Nice simplification, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116186/new/ https://reviews.llvm.org/D116186

[PATCH] D117304: [clang][dataflow] Remove TestingSupport's dependency on gtest

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:144 + auto *Lattice = + llvm::any_cast(&State.Lattice.Value); +

[PATCH] D116377: [libTooling] Adds more support for constructing object access expressions.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp:78 + returns(qualType(references(type())); + const auto SmartPo

[PATCH] D116377: [libTooling] Adds more support for constructing object access expressions.

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/SourceCodeBuildersTest.cpp:373 +TEST(SourceCodeBuildersTest, BuildAccessSmartPointer) { + testBuilder(buildAccess, "Smart x; x;", "x->"); +} ymandel wrote: > gribozavr2 wrote: > > This is a ca

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

gribozavr2 added a comment. LGTM but deferring approval to @xazax.hun . Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:30 +public: + void run(const ast_matchers::MatchFinder::MatchResult &Result) override { +const auto *Func = Result

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

This revision was automatically updated to reflect the committed changes. Closed by commit rG8dcaf3aa0bf2: [clang][dataflow] Implement a basic algorithm for dataflow analysis (authored by sgatev, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D116022: [clang][dataflow] Add support for terminating virtual destructors

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:50 + std::unique_ptr Cfg; + llvm::DenseMap StmtToBlock; +}; xazax.hun wrote: > There is a special class for this at `clang/lib/Analysis/CFGStmtMap.cpp`

[PATCH] D116022: [clang][dataflow] Add support for terminating virtual destructors

gribozavr2 added a comment. > How resilient is this pattern matching? Possibly it is not very robust. However it is very important for a certain style of precondition verification macros used by various Google projects. I can't find an exact open source copy, but they are generally similar to t

[PATCH] D116022: [clang][dataflow] Add support for noreturn destructor calls

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:196 + +class VirtualDestructorTest : public ::testing::Test { +protected: DYM "Noreturn" instead of "Virtual"? Repository: rG LLVM Github

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

gribozavr2 added a comment. Could you also update the switch statement in LLDB here: `llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:472` ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96771/new/ https://reviews.llvm.org/D9

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

gribozavr2 added a comment. Pushed the most minimal fix in https://github.com/llvm/llvm-project/commit/41454c30f6a38c3e107d857e63da0561610fd141, please double check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96771/new/ https://reviews.llvm.or

[PATCH] D100347: Fix the const overload of IgnoreExprNodes

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Thanks for the fix! Do you have commit access? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100347/new/ https://reviews.llvm.org/D10034

[PATCH] D100347: Fix the const overload of IgnoreExprNodes

This revision was automatically updated to reflect the committed changes. Closed by commit rG595c3758e470: Fix the const overload of IgnoreExprNodes (authored by d, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100347/new/ h

[PATCH] D100347: Fix the const overload of IgnoreExprNodes

gribozavr2 added a comment. > Trying arc land I realize I do not... Pushed as 595c3758e47042c876e7390d09642ea050ba6dbf . > it's also unclear to me where the tests for this are so I couldn't add a new > test, is that OK? Yes,

[PATCH] D108808: [clang-tidy] bugprone-infinite-loop: Fix false positives with volatile addresses.

gribozavr2 added a comment. > What happens for something like the following? Maybe it's also worth a test > case? WDYT about the test on line 614? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108808/new/ https://reviews.llvm.org/D108808

<    7   8   9   10   11   12