[PATCH] D88403: Migrate Declarators to use the List API

2020-09-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:593 +/// Shrink \p Range to a subrange that only contains tokens of a list. +ArrayRef shrinkToFitList(ArrayRef Range) { Comment at: clang/lib/Toolin

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

2020-10-05 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Google C++ style guide does not have this rule anymore. Thanks for the cleanup! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88831/new/

[PATCH] D88886: [Clang][unittests][NFC] Break up test in Callbacks.cpp

2020-10-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a reviewer: gribozavr2. gribozavr2 added a comment. Thank you! LGTM assuming you only moved the tests into separate cpp files (I didn't verify that the text is exactly the same). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88886: [Clang][unittests][NFC] Break up test in Callbacks.cpp

2020-10-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/CallbacksCommon.h:1 + +using namespace clang; Please add a license comment and all necessary includes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D88106: [SyntaxTree] Provide iterator-like functions for Lists

2020-10-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:211 + return element == Other.element && delimiter == Other.delimiter; +} }; Please also define `operator!=` even if it is not used yet. Comment

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

2020-10-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Tree.cpp:103 #ifndef NDEBUG - for (auto *N = New; N; N = N->getNextSibling()) { + for (auto *N = New; N; N = N->NextSibling) { assert(N->Parent == nullptr); eduucaldas wrote: > Through

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

2020-10-14 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/Tree.cpp:122 #endif + Node *&Begin = BeforeBegin ? BeforeBegin->NextSibling : FirstChild; + Could you move

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

2020-10-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
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/performance-unnecessary-copy-initialization.cpp:409 + +namespace std { + Could you add a

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

2020-11-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525 +return "; cannot be fixed because '" + Fixup + + "' is not a valid ide

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

2020-11-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525 +return "; cannot be fixed because '" + Fixup + + "' is not a valid identifier"; njames93 wrote: > gribozavr2 wrote: > > I don't think w

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

2020-11-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525 +return "; cannot be fixed because '" + Fixup + + "' is not a valid identifier"; njames93 wrote: > gribozavr2 wrote: > > njames93 wrote:

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

2020-10-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:409 + +namespace std { + flx wrote: > gribozavr2 wrote: > > Could you add a nested inline namespace to better imitate what de

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Any chance of adding a test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89608/new/ https://reviews.llvm.org/D89608 _

[PATCH] D88553: [clangd] Start using SyntaxTrees for folding ranges feature

2020-10-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:37 -// Recursively collects FoldingRange from a symbol and its children. -void collectFoldingRanges(DocumentSymbol Symbol, - std::vector &Result) { +FoldingRang

[PATCH] D88553: [clangd] Start using SyntaxTrees for folding ranges feature

2020-10-21 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:172-174 /// Find the first node with a corresponding role. Node *findChild(NodeRole R); + const Node *findChild(NodeRole R) const; eduucaldas wrote: > I think that make

[PATCH] D90127: [clang][NFC] Rearrange Comment Token and Lexer fields to reduce padding

2020-10-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/AST/CommentLexer.h:244 + /// command, including command marker. + SmallString<16> VerbatimBlockEndCommandName; + I'm not a fan of this change to `Lexer` because it breaks the grouping of fields:

[PATCH] D90127: [clang][NFC] Rearrange Comment Token and Lexer fields to reduce padding

2020-10-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/AST/CommentLexer.h:74 /// contains the length of the string that starts at TextPtr. unsigned IntVal; Coul

[PATCH] D90023: [Syntax] Add iterators over children of syntax trees.

2020-10-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:182 +/// The element, or nullptr if past-the-end. +NodeT *asPointer() const { return N; } + };

[PATCH] D90161: [SyntaxTree] Provide iterators for Lists

2020-10-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Could you add tests that verify the pairing of elements and delimiters? Comment at: clang/include/clang/Tooling/Syntax/Tree.h:214 + /// elements and delimiters are represented as null pointers. Below we give + /// examples of what this iteration wo

[PATCH] D90023: [Syntax] Add iterators over children of syntax trees.

2020-10-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:182 +/// The element, or nullptr if past-the-end. +NodeT *asPointer() const { return N; } + }; sammccall wrote: > gribozavr2 wrote: > > "nodePointer()" ? > I find thi

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

2020-10-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. > I am having a hard time to accept "this is how it is implemented in our fork" > as a technical argument. Besides, I am not sure how could the Clang community > benefit about being backward compatible with a specialized fork and thus > making superfluous complicatio

[PATCH] D90240: [SyntaxTree] Add reverse links to syntax Nodes.

2020-10-27 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:189 /// EXPECTS: Child->Role != Detached void prependChildLowLevel(Node *Child); friend class TreeBuilder; eduucaldas wrote: > Should we provide an `appendChildLowLev

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

2020-10-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. > Fair enough. But I don't think that Clang developers just copied the > implementation from GCC or from MSVC. No (nor we could copy the implementation for a number of reasons). However, we did have to be bug-for-bug compatible sometimes for compatibility. Reposito

[PATCH] D90240: [SyntaxTree] Add reverse links to syntax Nodes.

2020-10-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:188 + /// Similar but appends. + void appendChildLowLevel(Node *Child, NodeRole Role); + This is a complete nitpick, but could you put append before prepend? I think a lot

[PATCH] D104317: [libTooling][NFC] Refactor implemenation of Transformer Stencils to use standard OOP

2021-06-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:158 break; } +return (OpName + "(\"" + Id + "\")").str(); Please use spaces

[PATCH] D111190: Comment parsing: Complete list of Doxygen commands

2021-11-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Sorry for the delayed response! LGTM modulo the unit test comment. Please let me know if you don't have commit access and you need me to push on your behalf. Comment

[PATCH] D111262: Comment AST: Factor out function type extraction in DeclInfo::fill (NFC)

2021-11-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/Comment.cpp:337 + if (Kind == TypedefKind) +Kind = FunctionKind; ParamVars = FTL.getParams(); Phabr

[PATCH] D111264: Comment AST: Declare function pointer variables as functions

2021-11-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. I feel uneasy about claiming that variable decls of function type are FunctionKind for the purposes of comment parsing (even doing it for typedefs is questionable). It seems like a bet

[PATCH] D111266: Comment AST: Add support for variable templates

2021-11-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Sema/warn-documentation.cpp:1348 + /** + * functionPointerField + * "functionPointerTemplateMember" Repository: r

[PATCH] D113691: Comment AST: Recognize function-like objects via return type (NFC)

2021-11-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Nice, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113691/new/ https://reviews.llvm.org/D113691 __

[PATCH] D113690: Comment AST: Find out if function is variadic in DeclInfo::fill

2021-11-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Thank you for the cleanup! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113690/new/ https://reviews.llvm.org/D113690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

2021-11-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. I find the code more readable and robust when the `check*` function checks its applicability itself. After this refactoring, it is not so clear when each check functions applies, what are the correct conditions to call them. To ensure correct usage, probably we shoul

[PATCH] D113795: Comment Sema: Eliminate or factor out DeclInfo inspection (NFC)

2021-11-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Sorry, here I also find the old code to be more readable. - I don't see a problem with checks that are only used once. They are encapsulated in functions with meaningful names, making the code more readable. Compare `Sema::checkFunctionDeclVerbatimLine` before and af

[PATCH] D114011: Add a clang-transformer tutorial

2021-11-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/ClangTransformerTutorial.rst:52 +Patterns in Transformer are expressed with +`clang's AST matchers

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

2021-11-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/docs/DataFlowAnalysisIntro.md:24 +* [Introduction to Dataflow Analysis](https://www.youtube.com/watch?v=OROXJ9-wUQE) +* [Introduction to abstract interpretation](http://www.cs.tau.ac.il/~msagiv/courses/asv/absint-1.pdf). +

[PATCH] D114981: [clang-tidy] Allow disabling support for NOLINTBEGIN/NOLINTEND blocks.

2021-12-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Thank you for a quick workaround! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114981/new/ https://reviews.llvm.org/D114981 __

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

2021-12-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/docs/DataFlowAnalysisIntro.md:279 +``` +out = transfer(basic_block, join(in_1, in_2, ..., in_n)) +``` xazax.hun wrote: > gribozavr2 wrote: > > xazax.hun wrote: > > > While I agree that this is the general formul

[PATCH] D136224: [clang-tidy] Skip private default ctors in modernize-use-equals-default

2022-10-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:233 + auto IsPublicOrOutOfLineUntilCPP20 = + LangOpts.CPlusPlus20 || LangOpts.C

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103 + virtual void transferCFGElement(const CFGElement *Element, Lattice &L, + Environment &Env) {} + Instead of adding v

[PATCH] D131616: [clang][dataflow] Generalise match switch utility to other AST types and add a `CFGMatchSwitch` which currently handles `CFGStmt` and `CFGInitializer`.

2022-08-11 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. LGTM so far, please finish the patch and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131616/new/ https://reviews.llvm.org/D131616 ___ cfe-commits mailing list cfe-com

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144 llvm::Optional>>> -runDataflowAnalysis( +runDataflowAnalysisOnCFG( const C

[PATCH] D131616: [clang][dataflow] Generalise match switch utility to other AST types and add a `CFGMatchSwitch` which currently handles `CFGStmt` and `CFGInitializer`.

2022-08-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h:53-54 + template + CFGMatchSwitchBuilder CaseOfCFGStmt(MSMatcherT M, +

[PATCH] D131975: [clang][dataflow] Use llvm::is_contained()

2022-08-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added subscribers: martong, xazax.hun. Herald added a reviewer: NoQ. 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 https

[PATCH] D131975: [clang][dataflow] Use llvm::is_contained()

2022-08-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG941959d69de7: [clang][dataflow] Use llvm::is_contained() (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

2022-08-18 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:60 +/// Arguments for building the dataflow analysis. +template struct AnalysisArguments { + /// Input code that is analyzed. AnalysisInputs? ===

[PATCH] D132290: [clang-tidy] Skip unions in use-equals-default

2022-08-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:221 // Destructor. - Finder->addMatcher(cxxDestructorDecl(isDefinition()).bind(Sp

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
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:106 + /// `BlockStates` field which is only computed later during the analysis. + std::funct

[PATCH] D132745: [clang] Fix ambiguous use of `report_fatal_error`.

2022-08-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Basic/SanitizerSpecialCaseList.cpp:36 return SSCL; - llvm::report_fatal_error(Error); + llvm::report_fatal_error(llvm::StringRef(Erro

[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

2022-08-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:117 +llvm::DenseMap +getAnnotationLinesAndContent(AnalysisOutputs &AO); + (if possible) Comment at: clang/unittests/Analysis/FlowSensitive/Tes

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

2022-08-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp:124 + } + Result[Stmt] = Annotation; + Commen

[PATCH] D132756: [clang][dataflow] Refactor `TypeErasedDataflowAnalysisTest` - replace usage of the deprecated overload of `checkDataflow`.

2022-08-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
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:68 +const Environment &getEnvironmentAtAnnotation( +llvm::StringMap> &AnnotationStates, +

[PATCH] D133698: [clang][dataflow] SignAnalysis, edgeTransfer, branchTransfer

2022-09-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:127 + // Default implementation is a Noop. + virtual void branchTransfer(bool Branch, const Stmt *S, Lattice &L, + Environment &Env) {} -

[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

2022-10-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:157 + TopValue &createTop() { +return takeOwnership(std::make_unique()); + } Should we be creating a new top every time, or should it be a si

[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

2022-10-07 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:157 + TopValue &createTop() { +return takeOwnership(std::make_unique()); + } ymandel wrote: > xazax.hun wr

[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

2022-10-14 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp:69 -TEST(SolverTest, UnitConflict) { - ConstraintContext Ctx; Why delete this test? Repository: rG LLVM Github Monorepo

[PATCH] D140501: [clang][dataflow] Account for global variables in constructor initializers.

2022-12-21 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp:112 + const auto *Var = selectFirst("global", Results); + ASSERT_TRUE(Fun != nul

[PATCH] D140620: [clang] Migrate away from a deprecated Clang CFG factory function

2022-12-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: merrymeerkat. Herald added a subscriber: carlosgalvezp. Herald added a reviewer: njames93. Herald added a project: All. gribozavr requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscribe

[PATCH] D140620: [clang] Migrate away from a deprecated Clang CFG factory function

2022-12-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3a39b0ac1a72: [clang] Migrate away from a deprecated Clang CFG factory function (authored by gribozavr). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D140625: [clang] Remove deprecated ControlFlowContext::build()

2022-12-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: merrymeerkat. Herald added a reviewer: NoQ. 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 https://r

[PATCH] D140625: [clang] Remove deprecated ControlFlowContext::build()

2022-12-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd27386384a2a: [clang] Remove deprecated ControlFlowContext::build() (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D140694: [clang][dataflow] Only model struct fields that are used in the function being analyzed.

2022-12-27 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:147-148 + // Returns the subset of fields of `Type` that are referenced in th

[PATCH] D140703: [clang][dataflow] Unify `TransferOptions` and `DataflowAnalysisContext::Options`.

2022-12-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h:35 struct DataflowAnalysisOptions { - /// Options for the built-in transfer

[PATCH] D140753: [clang][dataflow] Check both operand's type in mergeDistinctValues

2022-12-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 requested changes to this revision. gribozavr2 added a comment. This revision now requires changes to proceed. Thank you for your contribution! While adding a conditional check fixes the crash, the problem's root cause must be deeper. Mismatched types indicate that one code path in da

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

2022-12-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. > With this change, unions are modeled exactly as structs (as far as I can > tell), which is unsound. Modeling just the storage of the union (but not the value) is sound. The first revision of the patch was a targeted fix that allowed us to continue maintaining the

[PATCH] D140753: [clang][dataflow] Fix crash when having boolean-to-integral casts.

2022-12-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:97-106 +// FIXME: This is sort of workaround. Since now we just ignore all (implicit) +

[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

2023-01-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added subscribers: martong, xazax.hun. Herald added a project: All. gribozavr requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In unit tests for concrete dataflow analyses we typically use the testonly

[PATCH] D141158: [OpenMP] Introduce '-f[no-]openmp-target-jit' flag to control JIT for offloading

2023-01-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. The newly added test fails when `tools/clang/include/clang/Config/config.h` is configured with `#define CLANG_DEFAULT_OPENMP_RUNTIME "libgomp"`. Please fix. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141158/n

[PATCH] D141158: [OpenMP] Introduce '-f[no-]openmp-target-jit' flag to control JIT for offloading

2023-01-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141158/new/ https://reviews.llvm.org/D141158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

2023-01-10 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. In D141283#4038703 , @tbaeder wrote: > When grepping `clang/test/` for "for char array is too long", I get a bunch > of other hits for this diagnostic, are the those tests not failing for you > (e.g. `clang/test/Sema/array-in

[PATCH] D141384: [clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

2023-01-10 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141384/new/ https://reviews.llvm.org/D141384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D137334: [clang][dataflow] Generalize custom comparison to return tri-value result.

2022-11-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:73-74 +/// Returns: +/// `Same`:`Val1` is equivalent to `Val2`, according

[PATCH] D137584: Add const information about AST nodes used to construct CFG elements.

2022-11-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3c50f0d879bf: Add const information about AST nodes used to construct CFG elements. (authored by merrymeerkat, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D142560: Allow getRawCommentForDecl to find comments in macros

2023-01-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8bb54da5da31: Allow getRawCommentForDecl to find comments in macros (authored by danakj, committed by gribozavr). Repository: rG LLVM Github Monor

[PATCH] D142637: A slightly more concise AST dump :)

2023-01-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/test/AST/ast-dump-attr-type.cpp:12 +// CHECK-NEXT:`-BuiltinType 0x{{[^ ]*}} 'int' +// CHECK-NOT: `-PointerType 0x564c31a07520 'int *' sammccall wrote: > Can you add a second example where the attri

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

2023-01-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Thank you for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142799/new/ https://reviews.llvm.org/D142799 ___ cfe-commits

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

2023-01-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/docs/ReleaseNotes.rst:60 `Issue 59446 `_. +- Fix crash when diagnosing incorrect usage of _Nullable involing alias + templates. This fixes

[PATCH] D141105: [OpenMP] Add support for '--offload-arch=native' to OpenMP offloading

2023-01-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. FYI: I fixed the problem in https://github.com/llvm/llvm-project/commit/0a11a1b1868dd2ab183c4313ccbfbe126e91ca08. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141105/new/ https://reviews.llvm.org/D141105 _

[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

2023-01-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 491003. gribozavr added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140859/new/ https://reviews.llvm.org/D140859 Files: clang/unittests/Analysis/FlowSensitive/TestingSu

[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

2023-01-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp:59 +if (SM.isPointWithin(Loc, BoundingRange.getBegin(), + BoundingRange.getEnd())) { + LineNumberToContent[SM.getPresumedLineNumber(Loc)] = ---

[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

2023-01-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc8b31da1ef0a: [clang][dataflow] Allow analyzing multiple functions in unit tests (authored by gribozavr). Repository: rG LLVM Github Monorepo CHA

[PATCH] D141709: [clang][dataflow] Fix bug in joining bool values.

2023-01-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141709/new/ https://reviews.llvm.org/D141709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D127745: [clang][dataflow] Rename `getPointeeLoc` to `getReferentLoc` for ReferenceValue.

2022-06-14 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG97d69cdaf324: [clang][dataflow] Rename `getPointeeLoc` to `getReferentLoc` for ReferenceValue. (authored by wyt, committed by gribozavr). Repository

[PATCH] D127886: [clang-tidy] Allow access to the SourceManager in clang-tidy checks

2022-06-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Could you provide more context? There seems to be plenty of ways to access the SourceManager from a ClangTidy check, for example, see `clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128060: [clang][dataflow] Extend flow condition in the body of a for loop

2022-06-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3577-3580 +ASSERT_THAT(Results, ElementsAre(Pair("if_branch_1", _), +

[PATCH] D128183: [clang][dataflow] Extend flow condition in the body of a do/while loop

2022-06-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3700 +EXPECT_TRUE(AfterLoopEnv.flowConditionImplies( +AfterLoopEnv.makeNot

[PATCH] D128363: [clang][dataflow] Implement functionality for flow condition variable substitution.

2022-06-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:163 +BoolValue &Val, +llvm::DenseMap &SubstitutionsCache) { + auto It = SubstitutionsCache.find(&Val); Could you refactor it into a free function?

[PATCH] D128401: [clang-tidy] Fixing a bug raising false alarms on static local variables in the Infinite Loop Checker

2022-06-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:159 +llvm::SmallSet &Callees) { +if (const CallExpr *Call

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

2022-06-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38 +template +llvm::DenseSet diagnoseCFG( +const ControlFlowContext &CFCtx, This function seems pretty general and not necessarily tied to diagnostics. WDYT

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

2022-06-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38-42 +llvm::DenseSet diagnoseCFG( +const ControlFlowContext &CFCtx, +std::vector> &BlockStates, +const Environment &Env, TypeErasedDataflowAnalysis &Analysis, +Di

[PATCH] D128467: [clang][dataflow] Allow MatchSwitch to return a value

2022-06-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:49 /// appropriate handler. -template -using MatchSwitch = std::function; +template +using MatchSwitch = std::function; WD

[PATCH] D128406: clang: Tweak behaviour of warn_empty_while_body and warn_empty_if_body

2022-06-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG37b881aa0bca: clang: Tweak behaviour of warn_empty_while_body and warn_empty_if_body (authored by gribozavr). Repository: rG LLVM Github Monorepo

[PATCH] D128401: [clang-tidy] Fixing a bug raising false alarms on static local variables in the Infinite Loop Checker

2022-06-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:184-185 +const Decl *Func) { + bool containsFunc = false; + bool overlap = false; + gribozavr2 wrote: > It seems like you missed this

[PATCH] D128446: [clang][dataflow] Use annotations for optional diagnostic tests

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1258 + + llvm::DenseSet AnnotationLines; + for

[PATCH] D128357: [clang][dataflow] Store flow condition constraints in a single `FlowConditionConstraints` map.

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfb88ea62602c: [clang][dataflow] Store flow condition constraints in a single… (authored by wyt, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D128519: [clang][dataflow] Move logic for creating implication and iff expressions into `DataflowAnalysisContext` from `DataflowEnvironment`.

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG00e9d53453ab: [clang][dataflow] Move logic for creating implication and iff expressions into… (authored by wyt, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D128519?vs=4397

[PATCH] D128520: [clang][dataflow] Refactor function that queries the solver for satisfiability checking.

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:224 + /// assignment for `Constraints` + bool checkUnsatisfiable(llvm::DenseSet Constraints) { +return querySolver(std::mov

[PATCH] D128520: [clang][dataflow] Refactor function that queries the solver for satisfiability checking.

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG42a7ddb428c9: [clang][dataflow] Refactor function that queries the solver for satisfiability… (authored by wyt, committed by gribozavr). Repository

[PATCH] D128521: [clang][dataflow] Implement functionality to compare if two boolean values are equivalent.

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0f65a3e61005: [clang][dataflow] Implement functionality to compare if two boolean values are… (authored by wyt, committed by gribozavr). Repository:

[PATCH] D128556: Make Objective-C++ match Objective-C's behavior on implicit ivar access when `self` is shadowed

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Please add a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128556/new/ https://reviews.llvm.org/D128556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D128363: [clang][dataflow] Implement functionality for flow condition variable substitution.

2022-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:209 + /// `Substitutions`, it will be substituted with the value it maps to. + BoolValue &buildAndSubstituteFlowCondition( +

<    4   5   6   7   8   9   10   11   12   >