[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] D55170: [clang-format]: Add NonEmptyParentheses spacing option

reuk updated this revision to Diff 191015. reuk added a comment. @MyDeveloperDay I'm sorry, you're absolutely correct. I'd got some other JUCE-related changes mixed up in this PR. Should be fixed now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz added a comment. I don't have commit access. It would be nice if someone could commit it in my place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@

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

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTTypeTraits.h:81 + /// \{ + /// Return the AST node kind of this ASTNodeKind. lebedev.ri wrote: > aaron.ballman wrote: > > These markings are a bit strange, can you explain them to me? > It

[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] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

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

[PATCH] D59318: [clang-tidy] add an overload for diag method that allows users to provide a diagnostic name rather than using the check name when building a diagnostic.

htl abandoned this revision. htl added a comment. In D59318#1430829 , @JonasToth wrote: > What is the reason you want this change to happen? I think this gives the > chance to create inconsistencies which we should avoid. Sorry this was a bit rushed. I

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

nridge added a comment. Great, thanks for the reviews! Could you commit the patch as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56370/new/ https://reviews.llvm.org/D56370 ___ cfe-commits mail

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

nridge added a comment. In D58880#1422494 , @kadircet wrote: > Also please have a look at D59083 , and make > sure it helps implement what you have in might and let me know if there's > anything missing. I verified that

[PATCH] D59407: [clangd] Add RelationSlab

nridge added a comment. In D59407#1430656 , @kadircet wrote: > I believe it makes sense to deduplicate SymbolIDs for RelationSlab. > Up until now, we mostly had only one occurence of a SymbolID in a Slab, but > RelationSlab does not follow that assumptio

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

lildmh created this revision. lildmh added reviewers: ABataev, hfinkel, Meinersbur, kkwli0. lildmh added a project: OpenMP. Herald added subscribers: cfe-commits, jdoerfert, guansong. Herald added a project: clang. This patch implements the code generation for OpenMP 5.0 declare mapper (user-defi

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

Tyker updated this revision to Diff 191031. Tyker added a comment. i added tests as you requested. i didn't add test for the codegen as it wasn't affected. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59467/new/ https://reviews.llvm.org/D59467 Files: clang/include/clang/Basic/Attr.

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

Tyker updated this revision to Diff 191034. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59467/new/ https://reviews.llvm.org/D59467 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Parse/Par

[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

Tyker updated this revision to Diff 191035. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59467/new/ https://reviews.llvm.org/D59467 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Parse/Par

[PATCH] D59407: [clangd] Add RelationSlab

nridge marked 7 inline comments as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/index/Index.h:43 +public: + using value_type = std::pair>; + using const_iterator = std::vector::const_iterator; kadircet wrote: > gribozavr wrote: > > `

[PATCH] D59407: [clangd] Add RelationSlab

nridge updated this revision to Diff 191036. nridge marked an inline comment as done. nridge added a comment. Herald added a subscriber: mgorny. Address review comments, except for the deduplication which is still under discussion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

aaron.ballman added a comment. I believe this has the incorrect modeling -- in the following code example, the attribute appertains to the substatement of the if statement, not the if statement itself: if (foo) [[likely]] { blah; } This falls out from: http://eel.is/c++draft/stmt.stmt

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1151 +def Likely : StmtAttr { + let Spellings = [CXX11<"", "likely", 201903>, CXX11<"clang", "likely">]; + let Documentation = [LikelyDocs]; I think this should have a C spelling

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

Tyker marked an inline comment as done. Tyker added a comment. I added the sema testes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59467/new/ https://reviews.llvm.org/D59467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

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

Tyker added a comment. 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 without warning or error ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59467/new

[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] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added inline comments. Comment at: include/clang/Format/Format.h:1578 /// \endcode bool SpaceBeforeCpp11BracedList; boolean here not enum see comment below Comment at: lib/Format/TokenAnnotator.cpp:2608 +(Style.Sp

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

reuk updated this revision to Diff 191050. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp lib/Format/TokenAnnotator.h unittests/Format/FormatTest.c

r356354 - Add testcase from bug 41079

Author: arsenm Date: Sun Mar 17 16:16:31 2019 New Revision: 356354 URL: http://llvm.org/viewvc/llvm-project?rev=356354&view=rev Log: Add testcase from bug 41079 Modified: cfe/trunk/test/CodeGen/builtin-expect.c Modified: cfe/trunk/test/CodeGen/builtin-expect.c URL: http://llvm.org/viewvc/ll

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

dcoughlin requested changes to this revision. dcoughlin added a comment. This revision now requires changes to proceed. I'm pretty worried about exposing this flag to end users. - Almost none of the options you've listed are user facing. Many represent options intended for use by static analyzer

[PATCH] D57860: [analyzer] Validate checker option names and values

dcoughlin requested changes to this revision. dcoughlin added a comment. This revision now requires changes to proceed. I think it would be better if the default for compatibility mode were 'true'. That way this change will be backwards compatible and clients who want to enforce stricter checkin

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

dcoughlin added a comment. This looks good to me. It is great to see this tested! Did you consider separating the checker options from the non-checker options in the config dumper output? That would probably be easier to read. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57922/new/ h

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

dcoughlin requested changes to this revision. dcoughlin added a comment. This revision now requires changes to proceed. I'm not sure we really want to add any more examples of plugins. Analyzer plugins aren't supported and likely never will be. We probably shouldn't give people false hope that a

[PATCH] D59296: [pp-trace] Delete -ignore and add generalized -callbacks

MaskRay added a comment. @aaron.ballman Does the release note look good now? 😊 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59296/new/ https://reviews.llvm.org/D59296 ___ cfe-commits mailing list cfe-c