[PATCH] D69223: WDocumentation: Implement the \anchor.

gribozavr2 added a comment. LGTM, feel free to push. Comment at: clang/lib/AST/TextNodeDumper.cpp:493 + case comments::InlineCommandComment::RenderAnchor: +OS << " RenderAnchor"; +break; Mordante wrote: > Mordante wrote: > > gribozavr2 wrote: > > > Ple

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

gribozavr2 added a comment. Updated version LGTM, thanks! Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:346 +/// static_assert(, ) +/// static_assert() +class StaticAssertDeclaration final : public Declaration { ilya-biryukov wrote: > gribozavr2 wrote:

[PATCH] D64573: [Syntax] Allow to mutate syntax trees

gribozavr2 added a comment. @Ka-Ka Thanks for bringing it to our attention! I fixed these warnings in: https://github.com/llvm/llvm-project/commit/9caf12e8495c1106dd3d1079892ce4f39f91b7d2 https://github.com/llvm/llvm-project/commit/73f423e739bcb9bee7b73f05d4bcd50782013a8c Repository: rG LLVM

[PATCH] D75632: Comment parsing: Treat \ref as inline command

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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

gribozavr2 added a comment. I can see it being useful for fix-its and source ranges, however I have a concern about it accidentally eating arguments that were supposed to go into a format string. For that, I second Aaron's suggestion to implement an extension for format strings. If you don't wa

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/Diagnostic.h:1298 + return DB; +}; + s/;// (and in functions below as well) Repository: rG LLVM Github

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Matchers.cpp:17 + +Matcher hasAnyListedName(std::vector Names) { + return Matcher(new HasNameMatcher(std::move(Names))); This matcher sounds generally useful. I think it would be be

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Matchers.cpp:17 + +Matcher hasAnyListedName(std::vector Names) { + return Matcher(new HasNameMatcher(std::move(Names))); njames93 wrote: > gribozavr2 wrote: > > This matcher sounds

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

gribozavr2 closed this revision. gribozavr2 added a comment. Superseded by https://reviews.llvm.org/D76001. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72072/new/ https://reviews.llvm.org/D72072 ___

[PATCH] D76001: [AST] Respect shouldTraversePostOrder when traversing type locs

This revision was automatically updated to reflect the committed changes. Closed by commit rGb720543926c7: [AST] Respect shouldTraversePostOrder when traversing type locs (authored by hlopko, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D76061: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/AST/DeclTest.cpp:125 + auto StarLoc = TL.getStarLoc().printToString(SM); + ASSERT_EQ(StarLoc, "input.cc:3:12"); +}

[PATCH] D76061: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/unittests/AST/SourceLocationTest.cpp:856 + + auto *D = selectFirst("vd", match(varDecl().bind("vd"), Ctx)); + ASSERT_TRUE(D != nullptr); s/D/VD/? (for VarDecl) Repository:

[PATCH] D76121: Modernize DeclTest

This revision was automatically updated to reflect the committed changes. Closed by commit rGdb1f40d1a16f: Modernize DeclTest (authored by hlopko, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76121/new/ https://reviews.llvm

[PATCH] D76061: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

This revision was automatically updated to reflect the committed changes. Closed by commit rGce79c4246919: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc (authored by hlopko, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D72073: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Superseded by https://reviews.llvm.org/D76061. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72073/new/ https://reviews.llvm.org/D72073

[PATCH] D76120: Refactor SourceLocationTest to `using namespace`

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGf8640737d476: Refactor SourceLocationTest to `using namespace` (authored by hlopko, committed by gribozavr). Changed prio

[PATCH] D72089: [Syntax] Build declarator nodes

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1210 + `-; + )txt"}, }; hlopko wrote: > gribozavr2 wrote: > > A few complex tests that combine multiple declarators would be nice > > (especially to test that the

[PATCH] D76220: [Syntax] Build declarator nodes

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:519 + +/// Parameter list for a function type. +/// E.g.: ... and a trailing return type, if the function has one. Comment at: clang/include/clang/Tooling/

[PATCH] D76220: [Syntax] Build declarator nodes

gribozavr2 accepted this revision. gribozavr2 added a comment. BTW, would be also nice to have tests for trailing return types not at the top level -- in a follow-up: `auto x(char a, auto (*b)(int) -> short) -> void;` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D72089: [Syntax] Build declarator nodes

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Superseded by https://reviews.llvm.org/D76220. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72089/new/ https://reviews.llvm.org/D72089

[PATCH] D76220: [Syntax] Build declarator nodes

This revision was automatically updated to reflect the committed changes. Closed by commit rG7d382dcd46a1: [Syntax] Build declarator nodes (authored by hlopko, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76220/new/ https:/

[PATCH] D72334: [Syntax] Build nodes for template declarations.

gribozavr2 added a comment. Superseded by https://reviews.llvm.org/D76346. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72334/new/ https://reviews.llvm.org/D72334 ___ cfe-commits mailing list cfe-comm

[PATCH] D76366: [Syntax] Split syntax tests

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:642 + +TEST_F(SyntaxTreeTest, UsingNamespaces) { + expectTreeDumpEqual( "using directive

[PATCH] D76346: [Syntax] Build template declaration nodes

This revision was automatically updated to reflect the committed changes. Closed by commit rGdd12826808f9: [Syntax] Build template declaration nodes (authored by hlopko, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D76346?vs=251095&id=251100#toc Repository: rG L

[PATCH] D76418: [Syntax] Build template declaration nodes

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG88bf9b3d26f0: [Syntax] Build template declaration nodes (authored by hlopko, committed by gribozavr). Changed prior to co

[PATCH] D76366: [Syntax] Split syntax tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGe9630630ffa2: [Syntax] Split syntax tests (authored by hlopko, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76366/new/ https://rev

[PATCH] D76433: [Syntax] Test both the default and windows target platforms in unittests

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:102 +std::vector Args = {"-target", Target.data(), + "-fsyn

[PATCH] D76433: [Syntax] Test both the default and windows target platforms in unittests

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd7300f717c1: [Syntax] Test both the default and windows target platforms in unittests (authored by hlopko, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D73869: [clang][AST] Add an AST matcher for deducedTemplateSpeializationType.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6043 +/// \endcode +/// \c deducedTemplateSpecializationType() matches the type in \c c. +extern const A

[PATCH] D73876: [clang-tidy] Fix a false positive about C++17 deduced class template types in unused-using-decl check.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls-cxx17.cpp:10 +}; +// Deduction hint (CTAD) +template Foo(T t) -> Foo; ---

[PATCH] D74468: [clang-tidy] No misc-definitions-in-headers warning on C++14 variable templates.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-definitions-in-headers.rst:86 + // OK: C++14 variable template is allowed. + template --

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. A test would be nice, but we don't have infrastructure for checking call ordering. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72072/n

[PATCH] D72089: [Syntax] Build declarator nodes

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:395 } + /// FIXME: use custom iterator instead of 'vector'. + std::vector declarators(); s/iterator/container/ ? Also unclear why a custom one should be used. I also t

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Thank you for factoring our this library! I briefly read the UninitializedValues analysis and I also think that this change is a no-op. Comment at: clang/include/cl

[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

gribozavr2 added a comment. Since I seem to be in the minority about thinking that this check does not pull its weight, I reviewed the code, and will LGTM and push once the few small issues are fixed. Comment at: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmet

[PATCH] D72274: [libTooling] Fix bug in Stencil handling of macro ranges

gribozavr2 added a comment. The only functional change that I see in this patch is in `clang/lib/Tooling/Transformer/Stencil.cpp`. However, I don't understand how that change in the (deprecated) selection() stencil can affect other stencils. Comment at: clang/unittests/Toolin

[PATCH] D72274: [libTooling] Fix bug in Stencil handling of macro ranges

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. In D72274#1826614 , @ymandel wrote: > In D72274#1826477 , @gribozavr2 > wrote: > > > The only function

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Transformer/SourceCode.cpp:128 + // First, lex to the current token (which is the last token of the range that + // we know to be deleted. Then, we process the first token separately from the + // rest based on c

[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

gribozavr2 added a comment. Let me know if you want me to commit this change for you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71001/new/ https://reviews.llvm.org/D71001 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

gribozavr2 added inline comments. Comment at: clang/include/clang-c/Documentation.h:383 CINDEX_LINKAGE -CXString clang_ParamCommandComment_getParamName(CXComment Comment); Mordante wrote: > gribozavr2 wrote: > > Please don't modify existing APIs in libclang --

[PATCH] D72097: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

gribozavr2 added a comment. Sorry, just getting back to this review. Your justification makes sense, and the patch LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72097/new/ https://reviews.llvm.org/D72097 __

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

gribozavr2 added a comment. I'd also appreciate if you updated the docs for the changes done in this patch. Comment at: clang/include/clang/AST/LifetimeAttr.h:22 + +/// This represents an abstract memory location that is used in the lifetime +/// contract representation. --

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186 +} + } + I think this logic should go into `isChanged()`, similarly to how it handles for loops today. Comment at: clang-tools-e

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186 +} + } + baloghadamsoftware wrote: > gribozavr2 wrote: > > I think this logic should go into `isChanged()`, similarly to how it > > handles for loo

[PATCH] D73300: [clang-tidy] Add library for clang-tidy main function

gribozavr2 added a reviewer: gribozavr2. gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.h:9 +/// +/// \file This file declares clang-tidy tool main

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/include/clang/AST/LifetimeAttr.h:22 + +/// This represents an abstract memory location that is used in the lifetime +/// contract representation. gribozavr2 wrote: >

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM! Please commit if you have commit access, or let me know if I should push it for you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ https://reviews.llvm.org/D

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

gribozavr2 added a comment. In D73270#1838883 , @njames93 wrote: > May not be one for this patch, but how does this check handle volatile loop > variables and cases where modification isn't visible in the context e.g. Should be OK, see "if (Var->getType

[PATCH] D73441: [clang-tidy] Fix bugprone-use-after-move when move is in noexcept operator

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:387 + unless(inDecltypeOrTemplateArg()), + unless(hasAncestor(cxxNoexceptExpr( .bind("call-move"); Quuxplusone wro

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

gribozavr2 added a comment. Please talk to Hans Wennborg about cherry-picking this change into the release. I think it is a safe change, if Hans needs that sort of review from someone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ htt

[PATCH] D73441: [clang-tidy] Fix bugprone-use-after-move when move is in noexcept operator

gribozavr2 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:1276 +namespace PR44667 { +#define REQUIRE(expr) (void)(expr); +struct S {}; njames93 wrote: > gribozavr2 wrote: > > Is the macro a necessary par

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

gribozavr2 added a comment. In D71966#1840957 , @Mordante wrote: > So if I understand correctly: > > - `getParamNameAsWritten` will become `getArgText` > - The `getParamName` will do the translation from the name in the > documentation to the name in the

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

gribozavr2 added a comment. Thank you for the contribution! I didn't review the code thoroughly yet, only the tests. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:29 + + bool isHeader() const { +return llvm::StringSwitch(Extension) -

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:29 + + bool isHeader() const { +return llvm::StringSwitch(Extension) njames93 wrote: > gribozavr2 wrote: > > I think we should consider any

[PATCH] D73441: [clang-tidy] Fix bugprone-use-after-move when move is in noexcept operator

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM with fixes to the test. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:1276 +namespace PR44667 { +#define REQUIRE(expr) (voi

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:75 + continue; +if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart)) + return; // Found a good candidate for matching decl ---

[PATCH] D144987: [clang][dataflow] Fix missed fields in field set construction.

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp:141 + }; + S foo(); + )cc"; Is `foo()` used for anything in this test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144987: [clang][dataflow] Fix missed fields in field set construction.

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp:141 + }; + S foo(); + )cc"; ymandel wrote: > gribozavr2 wrote: > > Is `foo()` used for anything in this test? > not that I can see. looks le

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:31 - /// there isn't one. - /// FIXME: Ensure that the result can't be null and return a const

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

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:399 +template +ValueT &getValueFromDecl(ASTContext &ASTCtx, const Environment &Env, +

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:35 + /// The result is guaranteed never to be null. + virtual const Environment *getEnvironment(const Stmt &S) const { +auto BlockIt = CFCt

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Logger.h:34 static std::unique_ptr textual(llvm::raw_ostream &); + // A logger that builds an HTML UI to inspect the analysis results. + // One file is written under the specified dir pe

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

gribozavr2 added a comment. Could you upload an updated sample HTML file? It is easier to review the HTML generation and javascript code when an example is available. Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:388 +std::unique_ptr flagLogger() {

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:34 +// the data into tags embedded in the HTML. (see inflate() in JS). +// +// SELECTION Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cp

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:19 +Function + + sammccall wrote: > gribozavr2 wrote: > > Could you add the file name and line number of the start location of the > > function? > > > > It might be h

[PATCH] D144480: [C++20][ClangTidy] Update the ClangTidy tests to also test in C++20 mode

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init-cxx98.cpp:2 // RUN: %check_clang_tidy -std=c++98 %s cppcoreguideli

[PATCH] D144730: [FlowSensitive][WIP] log analysis progress for debugging purposes

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Logger.h:22 + +// A logger is notified as the analysis progresses. +// It can produce a report of the analysis's findings and how it came to them. Elsewhere under Analysis/F

[PATCH] D153409: [clang][dataflow] Treat fields of anonymous records as being part of their parent.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:305 +if (Field->isAnonymousStructOrUnion()) + getFieldsFromClassHierarchy(Field->getType(), Fields); +else C

[PATCH] D153476: [dataflow] document & test determinism of formula structure

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp:169 + auto &BOr = A.makeOr(BY, BX); + auto &BEqual = A.makeEquals(BOr, BAnd); + EXPECT_EQ(Expected, llvm::to_string(B.getFormula(BEqual))); Shouldn't these lines

[PATCH] D153476: [dataflow] document & test determinism of formula structure

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp:165-169 + auto &BX = A.create(); + auto &BY = A.create(); + auto &BAnd = A.makeAnd(BX, BY); + auto &BOr = A.makeOr(BY, BX); + auto &BEqual = A.makeEquals(BOr, BAnd); --

[PATCH] D153491: [dataflow] Avoid copying environment

gribozavr2 added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153491/new/ https://reviews.llvm.org/D153491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D153960: [clang][dataflow] Implement support for pointers to members.

gribozavr2 requested changes to this revision. gribozavr2 added a comment. This revision now requires changes to proceed. Using PointerValue to model pointers to data members does not look right to me, because a pointer to data member is an offset within an object that we apply this pointer to,

[PATCH] D153852: [clang][dataflow] Initialize fields of anonymous records correctly.

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:356 + Member = cast(I); + MemberLoc = &cast(MemberLoc)->getChild(*Member);

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:183 + /// been stored in flow conditions. + Solver::Result querySolver(llvm::DenseSet Constraints); + sammccal

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/Formula.h:27 +/// +/// This often represents an assertion that is interesting to the analysis but +/// cann

[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

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:61 + /// Passing in the same formula will result in the same BoolValue. + /// FIXME: This canonic

[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Formula.h:71 + bool literal() const { +assert(kind() == Literal); castAsLiteral() ? Comment at: clang/unittests/A

[PATCH] D147698: [clang][dataflow] Add support for new and delete expressions.

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:315 + /// `D` must currently be associated with a value. + void unsetChild(const ValueDecl &D) { +auto It = Children.find(&D); Modifying already-created values

[PATCH] D147698: [clang][dataflow] Add support for new and delete expressions.

gribozavr2 added a comment. In D147698#4257698 , @mboehme wrote: > @gribozavr2 Just to make sure I understand you correctly here (before I make > any changes to the code): IIUC you recommend simply doing nothing on a delete > expression? (Your arguments

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:47-50 +for (tmpl of root.getElementsByTagName('template')) { + // Clear previously rendered te

[PATCH] D148377: [clang][dataflow] Drop optional model's dependency on libc++ internals.

gribozavr2 accepted this revision. gribozavr2 added a comment. Nice deduplication and compatibility fixes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148377/new/ https://reviews.llvm.org/D148377 ___ c

[PATCH] D151071: [clang][dataflow] Fix a null pointer crash in `computeBlockInputState()`.

gribozavr2 added a comment. LGTM too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151071/new/ https://reviews.llvm.org/D151071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D151192: [clang-tidy] have bugprone-unchecked-optional-access check boost::optional usage

gribozavr2 added a comment. Could you change the tests to cover the new case? They are here: `llvm-project/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151192/new/ https

[PATCH] D151192: [clang-tidy] have bugprone-unchecked-optional-access check boost::optional usage

gribozavr2 added a comment. The mock optional types in the unit test are just declarations of the API - they don't need any implementations (function or method bodies should be omitted). But the declarations of classes, methods, and functions should mirror the production header closely. There a

[PATCH] D151723: [clang] Remove unnecessary FALLTHROUGH markers

gribozavr created this revision. gribozavr added a reviewer: NoQ. Herald added subscribers: steakhal, martong. Herald added a project: All. gribozavr requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. They are redundant with the [[fallthrough]

[PATCH] D151725: [clang] Move dyn_cast's into if statements for readability

gribozavr created this revision. gribozavr added a reviewer: NoQ. Herald added subscribers: steakhal, martong. Herald added a project: All. gribozavr requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo htt

[PATCH] D151726: [clang] Remove unnecessary casts around Allocate function calls

gribozavr created this revision. gribozavr added a reviewer: NoQ. Herald added subscribers: steakhal, martong. Herald added a project: All. gribozavr requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo htt

[PATCH] D151727: [clang] Replace dyn_cast with cast in MemRegion::getMemorySpace

gribozavr created this revision. gribozavr added a reviewer: NoQ. Herald added subscribers: steakhal, martong. Herald added a project: All. gribozavr requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. MemRegion::getMemorySpace() is annotated w

[PATCH] D151726: [clang][analyzer][NFC] Remove unnecessary casts around Allocate function calls

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0da99ffe1afc: [clang][analyzer][NFC] Remove unnecessary casts around Allocate function calls (authored by gribozavr). Repository: rG LLVM Github M

[PATCH] D151723: [clang][analyzer][NFC] Remove unnecessary FALLTHROUGH markers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdaa95c7de5b7: [clang][analyzer][NFC] Remove unnecessary FALLTHROUGH markers (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D151725: [clang][analyzer][NFC] Move dyn_cast's into if statements for readability

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0989ce947e3d: [clang][analyzer][NFC] Move dyn_cast's into if statements for readability (authored by gribozavr). Repository: rG LLVM Github Monore

[PATCH] D151727: [clang][analyzer][NFC] Replace dyn_cast with cast in MemRegion::getMemorySpace

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8a40f89e2e93: [clang][analyzer][NFC] Replace dyn_cast with cast in MemRegion::getMemorySpace (authored by gribozavr). Repository: rG LLVM Github M

[PATCH] D151818: [clang][analyzer][NFC] Use the operator new directly with the `BumpPtrAllocator`

gribozavr created this revision. gribozavr added reviewers: NoQ, steakhal. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. gribozavr requested review of this revi

[PATCH] D151818: [clang][analyzer][NFC] Use the operator new directly with the `BumpPtrAllocator`

gribozavr updated this revision to Diff 527115. gribozavr added a comment. Format the edits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151818/new/ https://reviews.llvm.org/D151818 Files: clang/lib/Analysis/CFG.cpp clang/lib/StaticAnalyzer/C

[PATCH] D151818: [clang][analyzer][NFC] Use the operator new directly with the `BumpPtrAllocator`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7ebf64f7e934: [clang][analyzer][NFC] Use the operator new directly with the `BumpPtrAllocator` (authored by gribozavr). Repository: rG LLVM Github

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

gribozavr2 added a comment. LGTM still. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149144/new/ https://reviews.llvm.org/D149144 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D142991: [clang-tidy] Add --fix-mode and --nolint-prefix options

gribozavr2 added inline comments. Herald added a subscriber: PiotrZSL. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:58 + (Lexer::getIndentationForLine(Loc, Loc.getManager()) + "/* " + + NoLintPrefix + "NOLINTNEXTLINE(" + Error.DiagnosticName

[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h:1 +int abort() { return 0; } + There should b

[PATCH] D150352: [clang][dataflow] Don't analyze templated declarations.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:36 + /// `S` resides. `D.isTemplated()` must be false. + static llvm::Expected build(const Decl &D, Stmt &S, +

[PATCH] D150352: [clang][dataflow] Don't analyze templated declarations.

gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2537 -TEST(TransferTest, DerefDependentPtr) { std::string Code = R"( This `DerefDependentPtr` test was originally added in https://reviews.llvm.org/D117567

<    6   7   8   9   10   11   12   >