[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri added a comment. In D58894#1416691 , @djtodoro wrote: > @lebedev.ri Thanks for your comment! > > > Is there any way to model this more generically? > > I.e don't duplicate every naive matcher (ignoring possible presence of , > > op) with an var

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri added a comment. In D58894#1418251 , @djtodoro wrote: > @lebedev.ri I agree, thank you! I needed to be more precise in my previous > reply, sorry for that. I thought it will be (somehow) overhead if I change > existing, very basic, matchers.

[PATCH] D58979: [clang][OpenMP] Revert "OMPFlushClause is synthetic, no such clause exists"

lebedev.ri created this revision. lebedev.ri added reviewers: gribozavr, ABataev, rjmccall, aaron.ballman. lebedev.ri added a project: clang. Herald added subscribers: jdoerfert, arphaman, guansong, dylanmckay. This reverts rL352390 / D57280

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri added a comment. Nice, i like this! I think the test coverage is good. But what about other `equalsNode()` that you didn't change? Do some of them need this change too? Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:27 +AST_MATCHER_P(Expr, skipCommaOps, +

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri added inline comments. Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:27 +AST_MATCHER_P(Expr, skipCommaOps, + ast_matchers::internal::Matcher, InnerMatcher) { djtodoro wrote: > lebedev.ri wrote: > > (Can anyone suggest a better name ma

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Anyways, this looks good in this state. Thank you for working on this! If you don't intend to immediately work on fixing the rest of `,` cases here, *please* do file a bug so it won't g

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri accepted this revision. lebedev.ri added a comment. Still LG. I'm sure you have verified that there is test coverage for all the newly-supported cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58894/new/ https://reviews.llvm.org/D58894 ___

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

lebedev.ri added a comment. Thank you for working on this! This looks questionable to me. Is this based on some coding standard? Are there any numbers on the false-positive rate of theck? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59103/new/ htt

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/IncompleteComparisonOperatorCheck.cpp:242-243 +diag(Operator->getBeginLoc(), "incomplete comparison operator"); +Lhs->diagFields(*this, Operator->getBeginLoc()); +Rhs->diagFields(*this, Operator->getBeg

[PATCH] D33029: [clang-format] add option for dangling parenthesis

lebedev.ri added a comment. In D33029#1423944 , @MyDeveloperDay wrote: > Adding the unit tests lets us see how this option will work in various cases, > it will let us understand that its not breaking anything else. > > I personally don't like to see rev

[PATCH] D59196: [NFC][clang][PCH] ASTStmtReader::VisitStmt(): fixup faulty assert.

lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, rtrieu, riccibruno. lebedev.ri added a project: clang. While this is NFC - `NumStmtFields` **is** currently `0`, the assert is faulty. It's not checking that `VisitStmt()` is called when `getIdx()` returns `NumStmtFields`. It

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S);`

lebedev.ri created this revision. lebedev.ri added reviewers: arphaman, sammccall, smeenai, aprantl, rsmith. lebedev.ri added a project: clang. These ObjC AST classes inherit from Stmt, but don't call `VisitStmt(S);`. Some were founded with help of existing tests (with `NumStmtFields` bumped to `

[PATCH] D59214: [private][clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri created this revision. lebedev.ri added reviewers: ABataev, rjmccall, hfinkel, rsmith, riccibruno, gribozavr. lebedev.ri added projects: clang, OpenMP. Herald added subscribers: jdoerfert, jfb, guansong. https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf, page

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

lebedev.ri added a comment. In D59197#1424644 , @aprantl wrote: > How can this change be NFC? `VisitStmt()` is empty currently, there is currently no data serialized by the `Stmt` itself. D59214 changes that. Reposito

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

lebedev.ri added a comment. In D59197#1424832 , @jordan_rose wrote: > I'm no Clang serialization expert but this makes sense to me. It's consistent > with all the other statement visitor methods. Thank you. Repository: rC Clang CHANGES SINCE LAST A

[PATCH] D59118: creduce script for clang crashes

lebedev.ri added a comment. The new diffs should not be relative to the previously uploaded diff, but to the git master/svn trunk. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59118/new/ https://reviews.llvm.org/D59118 ___ cfe-commits mai

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added subscribers: steveire, aaron.ballman. lebedev.ri added a comment. Thanks for taking a look, some replies. Comment at: include/clang/AST/StmtOpenMP.h:269 + /// or if this is an OpenMP stand-alone directive returns `None`. + llvm::Optional getStructuredBlock()

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:335 + llvm::Optional getStructuredBlockImpl() const { +return const_cast(getInnermostCapturedStmt()->getCapturedStmt()); ABataev wrote: > lebedev.ri wrote: > > ABataev wrote: > >

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: test/AST/ast-dump-openmp-atomic.c:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -ast-dump %s | FileCheck -strict-whitespace -implicit-check-not=openmp_structured_block %s + gribozavr wrote: > lebedev.

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:335 + llvm::Optional getStructuredBlockImpl() const { +return const_cast(getInnermostCapturedStmt()->getCapturedStmt()); lebedev.ri wrote: > ABataev wrote: > > lebedev.ri wrote: >

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:335 + llvm::Optional getStructuredBlockImpl() const { +return const_cast(getInnermostCapturedStmt()->getCapturedStmt()); ABataev wrote: > lebedev.ri wrote: > > lebedev.ri wrote: >

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

lebedev.ri added a comment. Herald added a subscriber: jdoerfert. Herald added a project: clang. In D36836#1021863 , @chandlerc wrote: > In D36836#931995 , @lebedev.ri wrote: > > > - Rebased > > - As advised by @aaro

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:335 + llvm::Optional getStructuredBlockImpl() const { +return const_cast(getInnermostCapturedStmt()->getCapturedStmt()); lebedev.ri wrote: > ABataev wrote: > > lebedev.ri wrote: >

[PATCH] D59306: [NFC][clang][astdump] Some baseline tests for OpenMP

lebedev.ri created this revision. lebedev.ri added reviewers: ABataev, aaron.ballman, stephenkelly. lebedev.ri added projects: clang, OpenMP. Herald added subscribers: jdoerfert, jfb, guansong. lebedev.ri added a child revision: D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[PATCH] D59048: Add AIX Target Info

lebedev.ri added inline comments. Comment at: test/Headers/max_align.c:1 +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics hubert.reinterpretcast wrote: > We may need to explicitly specify C11. It also seems that we should XFAIL > Windows

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

lebedev.ri added a comment. Please always upload all patches with full context (`=U9`) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:55 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + UseLegacyFunction(Options.getLocalOrGlobal("UseLegacyFunction", false)) {} I'm not sure it m

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added a comment. In D59214#1429384 , @riccibruno wrote: > IIRC, last time I looked only 4 statement/expression classes currently have > some abbreviation defined. Yep, that is what i'm seeing in this diff. In D59214#1429384

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added a comment. In D59214#1429422 , @riccibruno wrote: > In D59214#1429400 , @lebedev.ri > wrote: > > > In D59214#1429384 , @riccibruno > > wrote: > > > > > IIR

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

lebedev.ri added a comment. In D59336#1429564 , @aaron.ballman wrote: > This is missing test cases that demonstrate the behavior is what we expect it > to be for ObjC++ code vs C++ code. Looks like phab has again consumed the email comments. Might be a

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

lebedev.ri added a comment. Starting to look good i think. Comment at: docs/ReleaseNotes.rst:88 +- New alias :doc:`abseil-make-unique + ` to :doc:`modernize-make-unique Also add a new entry about the new option for `modernize-make-unique`. CHANGES SINCE LA

[PATCH] D59196: [NFC][clang][PCH] ASTStmtReader::VisitStmt(): fixup faulty assert.

lebedev.ri added a reviewer: dexonsmith. lebedev.ri added a comment. In D59196#1429393 , @riccibruno wrote: > I am not an expert in the serialization code (just did some modifications), > but this seems reasonable to me. That's my thoughts too.. Unless

[PATCH] D59196: [NFC][clang][PCH] ASTStmtReader::VisitStmt(): fixup faulty assert.

lebedev.ri abandoned this revision. lebedev.ri added a comment. In D59196#1431531 , @riccibruno wrote: > In D59196#1431473 , @lebedev.ri > wrote: > > > In D59196#1429393 , @

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:274 + /// Prerequisite: Executable Directive must not be Standalone directive. + Stmt *getStructuredBlock() const; }; riccibruno wrote: > This is not const-correct. The const-qualifie

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: lib/AST/StmtOpenMP.cpp:40 + if (auto *LD = dyn_cast(this)) +return LD->getBody(); + return getInnermostCapturedStmt()->getCapturedStmt(); @riccibruno `getBody()` exists in `const`-only variant `getInnermostCaptu

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: lib/AST/StmtOpenMP.cpp:40 + if (auto *LD = dyn_cast(this)) +return LD->getBody(); + return getInnermostCapturedStmt()->getCapturedStmt(); riccibruno wrote: > lebedev.ri wrote: > > @riccibruno > > `getBody()` exi

[PATCH] D59453: [ASTMatchers][OpenMP] Add base ompExecutableDirective() matcher.

lebedev.ri created this revision. lebedev.ri added reviewers: gribozavr, aaron.ballman, JonasToth, george.karpenkov. lebedev.ri added a project: clang. Herald added subscribers: jdoerfert, guansong. Split off from D57113 . Repository: rC Clang https://reviews

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri updated this revision to Diff 190965. lebedev.ri marked 3 inline comments as done. lebedev.ri retitled this revision from "[ASTTypeTraits] OMPClause handling" to "[ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling". lebedev.ri edited the summary of this revision. lebedev.ri added r

[PATCH] D57571: [clang-tidy] A new OpenMP module

lebedev.ri updated this revision to Diff 190972. lebedev.ri added a comment. Herald added a subscriber: jdoerfert. Rebased, no changes. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57571/new/ https://reviews.llvm.org/D57571 Files: clang-tidy/CMa

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

lebedev.ri updated this revision to Diff 190973. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Rebased, NFC. Moved matchers into D59453 +D57112 . Repository: rCTE Clang Tools Extra CHANGES SINCE L

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri created this revision. lebedev.ri added reviewers: gribozavr, aaron.ballman, JonasToth, george.karpenkov. lebedev.ri added projects: clang, OpenMP. Herald added subscribers: jdoerfert, guansong. Exposes the interface being added in D59214 for ASTMatch

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: include/clang/AST/ASTTypeTraits.h:81 + /// \{ + /// Return the AST node kind of this ASTNodeKind. aaron.ballman wrote: > These markings are a bit strange, can you expla

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6421 +/// +/// Prerequisite: the executable directive must not be standalone directive. +/// aaron.ballman wrote: > What happens if this prereq is not met? Does the matcher retu

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, baloghadamsoftware. lebedev.ri added projects: clang-tools-extra, OpenMP. Herald added subscribers: jdoerfert, guansong, rnkovacs, xazax.hun, mgorny. Herald added a project: clang. Finally, we are here! Analyz

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6421 +/// +/// Prerequisite: the executable directive must not be standalone directive. +/// lebedev.ri wrote: > aaron.ballman wrote: > > What happens if this prereq is not met?

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

lebedev.ri added a comment. Please always upload all patches with full context (`-U9`) Misses tests (sema, ast-dump, codegen) Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164 +def err_likely_attr_invalid_placement : Error< + "likely annotation can't apear

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

lebedev.ri added a comment. I think this looks good now, one nit about test coverage. Did you run this through your buildbot, any issues? Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bo

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

lebedev.ri added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:733 + StringRef T = "template void f(T v) \n"; + StringRef S = "{ T target = v; }"; + auto Cat = [&T](StringRef S) { return (T + S).str(); }; lebedev.ri wrote: > JonasToth wro

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri marked 3 inline comments as done. lebedev.ri added inline comments. Comment at: include/clang/AST/ASTTypeTraits.h:81 + /// \{ + /// Return the AST node kind of this ASTNodeKind. aaron.ballman wrote: > lebedev.ri wrote: > > aaron.ballman wrote: > >

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

lebedev.ri added a comment. In D59467#1432587 , @Tyker wrote: > i added tests as you requested. i didn't add test for the codegen as it > wasn't affected. Ah right, there wasn't some other clang-specific spelling for this already it seems, so indeed co

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

lebedev.ri added a comment. In D59467#1432675 , @Tyker wrote: > if likely/unlikely can be applied to any statement or label what should be > generated when it is applied to a statement that don't have any conditional > branch ? should it be ignored witho

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

lebedev.ri added a comment. In D59467#1432675 , @Tyker wrote: > if likely/unlikely can be applied to any statement or label what should be > generated when it is applied to a statement that don't have any conditional > branch ? should it be ignored witho

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri marked 4 inline comments as done. lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said m

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri marked 5 inline comments as done. lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said m

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri marked 3 inline comments as done. lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said m

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added a comment. Before i do address any further bike^W review notes in other reviews in this path stack, i want to finish with this review. If **this** change doesn't stick, rest is pointless waste of everyone's time. @ABataev please do tell if there is any outstanding issues here. I

[PATCH] D59509: Make static constructors + destructors minsize + cold (except for in -O0)

lebedev.ri added inline comments. Comment at: clang/test/CodeGen/static-attr.cpp:4 + +// WITHOUT-NOT: cold minsize noinline +// WITH: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]]] This is fragile, it may have false-negative if they appear in other

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri added a comment. In D59466#1434841 , @baloghadamsoftware wrote: > Great work! Thank you! I only have minor comment: did you consider moving the > refactoring of `ExceptionAnalyzer` into a separate (prerequisite) patch? Yes, absolutely, that

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

lebedev.ri added a comment. In D59135#1422822 , @thorsten-klein wrote: > Sorry for delay. I am currently not able to build master (Error: "sort" is > not a member of "llvm"). > And if I try to build 6.0.1-rc3 I can build clang-tidy but I am not able to

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64 +++i; +})"; + ASSERT_TRUE( gribozavr wrote: > Same comment as in the other patch -- I would prefer that the source is > inlined into the ASSERT_TRUE, with implicit string

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/Stmt.h:102 +/// This bit is set only for the Stmt's that are the structured-block +/// of OpeMP [non-standalone] executable directives. +/// I.e. those returned by OMPExecutableDirective::getStructuredBlo

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

lebedev.ri added inline comments. Comment at: include/clang/AST/Stmt.h:102 +/// This bit is set only for the Stmt's that are the structured-block +/// of OpeMP [non-standalone] executable directives. +/// I.e. those returned by OMPExecutableDirective::getStructuredBlo

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

lebedev.ri added a reviewer: ABataev. lebedev.ri added a comment. Interesting. What happens if libomp is not being built? What happens if libomp is not present? Or more generally, why does it even matter whether there is runtime or not, this only does paring+sema, no codegen/execution. Reposit

[PATCH] D59453: [ASTMatchers][OpenMP] Add base ompExecutableDirective() matcher.

lebedev.ri updated this revision to Diff 191674. lebedev.ri added a comment. Rebased, addressed nits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59453/new/ https://reviews.llvm.org/D59453 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri updated this revision to Diff 191676. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. Rebased, addressed all(?) nits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59463/new/ https://reviews.llvm.org/D59463 Files: docs/LibAS

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said matcher, + // and convert it to the OpenMPCla

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri updated this revision to Diff 191678. lebedev.ri marked 18 inline comments as done. lebedev.ri added a comment. Rebased, addressed all(?) nits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D57112 Files: docs/LibA

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri added a comment. @gribozavr thank you for the review! @aaron.ballman any further comments? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D57112 ___ cfe-commits mailing l

[PATCH] D59453: [ASTMatchers][OpenMP] Add base ompExecutableDirective() matcher.

lebedev.ri added a comment. @aaron.ballman @gribozavr thank you for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59453/new/ https://reviews.llvm.org/D59453 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri added a comment. @gribozavr thank you for the review! @aaron.ballman any comments? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59463/new/ https://reviews.llvm.org/D59463 ___ cfe-commits mailing list cfe-

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri updated this revision to Diff 191685. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Address @aaron.ballman comment nit. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D57112 Files: docs/L

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri updated this revision to Diff 191697. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Comment nit. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D57112 Files: docs/LibASTMatchersReference.

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6402 +/// +/// ``ompExecutableDirective(hasClause(anything()))`` matches +/// ``omp parallel default(none)``. aaron.ballman wrote: > hasAnyClause() whoops, thanks. Repository:

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

lebedev.ri added a comment. In D57112#1437954 , @aaron.ballman wrote: > LGTM Great, thank you for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D57112

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6444-6445 + internal::Matcher, InnerMatcher) { + if (isStandaloneDirective().matches(Node, Finder, Builder)) +return false; // Standalone directives have no structured blocks.

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

lebedev.ri updated this revision to Diff 191701. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Last nit. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59463/new/ https://reviews.llvm.org/D59463 Files: docs/LibASTMatchersReference.htm

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri created this revision. lebedev.ri added reviewers: baloghadamsoftware, JonasToth, gribozavr. lebedev.ri added a project: clang-tools-extra. Herald added a subscriber: rnkovacs. Herald added a project: clang. D59466 wants to analyse the `Stmt`, and `Exc

[PATCH] D57571: [clang-tidy] A new OpenMP module

lebedev.ri updated this revision to Diff 191724. lebedev.ri added a comment. Rebased, NFC. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57571/new/ https://reviews.llvm.org/D57571 Files: clang-tidy/CMakeLists.txt clang-tidy/ClangTidyForceLinker

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri updated this revision to Diff 191727. lebedev.ri added a reviewer: gribozavr. lebedev.ri added a comment. Rebased, NFC. Split base ExceptionEscapeCheck refactoring into D59650 . Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https:/

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

lebedev.ri updated this revision to Diff 191726. lebedev.ri marked 4 inline comments as done. lebedev.ri edited the summary of this revision. lebedev.ri added a reviewer: gribozavr. lebedev.ri added a comment. Rebased, NFC. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https:

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri added a comment. In D59650#1438515 , @JonasToth wrote: > Why not having normal overloads? The analysis for `Stmt` is implemented with > the private methods. Explicit template specialization is a bit overkill and > so easily understood (but not

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri updated this revision to Diff 191783. lebedev.ri added a comment. Rebased for D59650 changes, NFC. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59466/new/ https://reviews.llvm.org/D59466 Files: clang-

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri updated this revision to Diff 191782. lebedev.ri added a comment. Keep templated function out of the public interface. In D59650#1438603 , @JonasToth wrote: > > Looks like pointless code duplication that is easily avoidable. > > True, but I wou

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoilerplate(const T *Node) { + ExceptionInfo ExceptionList; Please

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri updated this revision to Diff 191870. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. Address some nits. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59466/new/ https://reviews.llvm.org/D59466 Files: clang-tid

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri added inline comments. Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53 + Finder->addMatcher(ompExecutableDirective( + unless(isStandaloneDirective()), + hasStructuredBlock(stmt().bind("structured-block")))

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri updated this revision to Diff 191868. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. Address some nits. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59650/new/ https://reviews.llvm.org/D59650 Files: clang-tid

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

lebedev.ri updated this revision to Diff 191869. lebedev.ri added a comment. Address some nits. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57113/new/ https://reviews.llvm.org/D57113 Files: clang-tidy/openmp/CMakeLists.txt clang-tidy/openmp/O

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoilerplate(const T *Node) { + ExceptionInfo ExceptionList; gribozavr wrote: > JonasToth wrote: > > lebedev.ri w

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri marked an inline comment as done. lebedev.ri added a comment. @gribozavr thank you for the review! @baloghadamsoftware any comments? Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoilerplate(const

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoilerplate(const T *Node) { + ExceptionInfo ExceptionList; griboza

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri updated this revision to Diff 191892. lebedev.ri marked 15 inline comments as done. lebedev.ri added a comment. Addressed some nits. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59466/new/ https://reviews.llvm.org/D59466 Files: clang-

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri added inline comments. Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53 + Finder->addMatcher(ompExecutableDirective( + unless(isStandaloneDirective()), + hasStructuredBlock(stmt().bind("structured-block")))

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

lebedev.ri added a comment. In D55170#1436732 , @MyDeveloperDay wrote: > > The cost is financial, as it's developer time, which costs real money to > > companies. In the end, to support this, people like myself who are doing > > this as part of their jo

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:10-11 +As per the OpenMP specification, structured block is an executable statement, +possibly compound, with a single entry at the top and a single exit at the +bottom. Which means,

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri updated this revision to Diff 191911. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. Last few nits. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59466/new/ https://reviews.llvm.org/D59466 Files: clang-tidy/op

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:19 + +.. code-block:: c++ + aaron.ballman wrote: > This is a *lot* of example text -- are you sure all of it adds value, or can > some of it be removed? Right, let's

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

lebedev.ri updated this revision to Diff 191918. lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. Make diags less grammatically correct, reduce docs. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57113/new/ https://reviews.ll

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

lebedev.ri added a comment. I'm gonna go with post-commit review here, if @baloghadamsoftware will have any notes, although this now looks rather uncontroversial. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59650/new/ https://reviews.llvm.org/D59

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

lebedev.ri added a comment. In D59466#1439880 , @aaron.ballman wrote: > Aside from a docs nit, LGTM! Thank you for the review! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59466/new/ https://reviews.llvm

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