[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-02 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11015 + "coroutine_traits defined in std::experimental namespace is deprecated. " + "Considering update libcxx and include instead of .">, + InGroup;

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-10-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Hi @ChuanqiXu sorry for what might be naive questions, but just to make sure I understand the context of this patch correctly: This patch fixes the look up of the `coroutine_traits`, so that clang now search both the `std` and the `std::experimental` namespace.

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 396361. avogelsgesang marked 4 inline comments as done. avogelsgesang added a comment. Thank you for the quick review, @HazardyKnusperkeks! I addressed most of your comments by fixing my code. In two cases, I think there was a misunderstanding and I

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:4003 +**TemplateArgumentKeyword** (``TemplateArgumentStyle``) :versionbadge:`clang-format 14` + Keyword to use for template arguments (``class`` or ``typename``)

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-26 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 396237. avogelsgesang added a comment. Fix code formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116290/new/ https://reviews.llvm.org/D116290 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-26 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added a subscriber: mgorny. avogelsgesang requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Template argument types can be introduced using either the `class` or the `typename` argument. While

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-26 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added reviewers: MyDeveloperDay, HazardyKnusperkeks, aaron.ballman, curdeius, sammccall, owenpan. avogelsgesang added a comment. Adding reviewers from D69764 to this change because I think it falls into the same category: It replaces tokens and

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 396403. avogelsgesang marked an inline comment as done. avogelsgesang added a comment. Fix namespace comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116290/new/ https://reviews.llvm.org/D116290

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang/include/clang/Format/Format.h:3691 + /// COULD lead to incorrect code formatting due to incorrect decisions made + /// due to clang-formats lack of complete semantic information. As such extra + /// care should be

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:36 +} // end namespace format +} // end namespace clang + avogelsgesang wrote: > curdeius wrote: > > Please make the comments consistent with other files. >

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 396401. avogelsgesang marked 7 inline comments as done. avogelsgesang added a comment. Thank you for your review, @curdeius! I updated my change to address most of your comments. Regarding "KeepTemplateTemplateKW", I think we had a misunderstanding

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

2021-11-15 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added subscribers: ymandel, whisperity, aaron.ballman. avogelsgesang added a comment. @ymandel, @whisperity, @aaron.ballman could one of you review this/point me in the direction of a good reviewer for this change? (Sorry for the spam - I am new to the LLVM project, and I guess I

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-15 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 387318. avogelsgesang added a comment. Update docs to also mention `container.find() != container.end()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 Files:

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-15 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @xazax.hun / @whisperity could you review this change for me, please :) Or, alternatively: can you direct me to somebody who could review? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-17 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 387926. avogelsgesang marked an inline comment as not done. avogelsgesang added a comment. Fix test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 Files:

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-11-11 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 386485. avogelsgesang added a comment. Don't update WorkingDir through local reference; as requested @ymandel and @hokein Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112647/new/

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-11-11 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Thanks for the reviews, @ymandel and @hokein! What are the next steps to get this landed? (I am a first time contributor and have no commit access) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112647/new/

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-16 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 387698. avogelsgesang marked 11 inline comments as done. avogelsgesang added a comment. Address review comments by @whisperity: - "containment" -> "membership" - "C++ 20" -> "C++20" - double-backticks in rst files - additonal test cases (both positive

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-16 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang marked 2 inline comments as not done. avogelsgesang added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp:105-108 + const auto *PositiveCheck = Result.Nodes.getNodeAs("positive"); + const auto *NegativeCheck =

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-26 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 390085. avogelsgesang marked 7 inline comments as done. avogelsgesang added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-26 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp:189 +// NO-WARNING. +// CHECK-FIXES: if (MyMap.count(0)) +return nullptr; whisperity wrote: > If a fix is not generated,

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-11-02 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a subscriber: alexfh. avogelsgesang added a comment. >> Those relative paths are meant to be resolved relative to the corresponding >> build directory. > > Is this behavior documented somewhere? I couldn't find this documented anywhere. My assumption is based on behavior

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-02 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. > [...] If we want to follow container-size-empty's convention, we should > include the replaced method in the name [...] Note that this commit is slightly different from `container-size-empty`: it doesn't only replace `count(...)` by `contains(...)` but also

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-11-02 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 384239. avogelsgesang marked 2 inline comments as done. avogelsgesang added a comment. Use `llvm::Optional` instead of pointer as suggested by @ymandel Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-10-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added reviewers: ymandel, njames93. avogelsgesang added a comment. Adding reviewers based on `git log` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112647/new/ https://reviews.llvm.org/D112647

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-10-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added a subscriber: carlosgalvezp. avogelsgesang requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. The fixes from the YAML file can refer to relative paths. Those relative paths are

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-10-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added subscribers: carlosgalvezp, xazax.hun, mgorny. avogelsgesang requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This commit introduces a new check `readability-container-contains`

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

2021-10-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a reviewer: alexfh. avogelsgesang added a subscriber: alexfh. avogelsgesang added a comment. not sure whom to add as a reviewer. According to `git log` this check wasn't changed for a while... Adding @alexfh based on `CODE_OWNERS.TXT` Repository: rG LLVM Github Monorepo

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

2021-10-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added subscribers: carlosgalvezp, xazax.hun. avogelsgesang requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This commit improves the fix-its of `modernize-pass-by-value` in two ways:

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-10-27 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 382741. avogelsgesang added a comment. Remove unrelated changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112647/new/ https://reviews.llvm.org/D112647 Files:

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-10-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. > Why readability-, if the intent is to make users move to a newer API? My line of thinking was: 1. The very similar `readability-container-size-empty` pass is also a readability pass. 2. The main reason why I want people to use `contains` over `count` is because

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-10-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang marked 2 inline comments as done. avogelsgesang added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:341-343 + `cert-exp42-c `_, `bugprone-suspicious-memory-comparison `_, `cert-fio38-c `_, `misc-non-copyable-objects `_, +

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-10-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:341-343 + `cert-exp42-c `_, `bugprone-suspicious-memory-comparison `_, `cert-fio38-c `_, `misc-non-copyable-objects `_, + `cert-flp37-c `_,

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-10-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 382982. avogelsgesang added a comment. Fix formatting; remove unrelated changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 Files:

[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

2021-11-09 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @alexfh, @ymandel: Do you know who could review this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112647/new/ https://reviews.llvm.org/D112647 ___ cfe-commits

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-09 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. gentle ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-12-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang marked 2 inline comments as done. avogelsgesang added a comment. Thanks for the instructions on how to build the documentation! I fixed a build issue in the docs (incorrect length of the table footer) and updated the wording slightly I am not planning any additional changes. If

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

2021-12-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:198 +for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) { + auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc(); + auto RefTL =

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

2021-12-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 391059. avogelsgesang marked 4 inline comments as done. avogelsgesang added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112648/new/ https://reviews.llvm.org/D112648

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-12-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 391075. avogelsgesang added a comment. Fix build error in documentation and update wording Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 Files:

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

2021-12-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 391121. avogelsgesang marked an inline comment as done. avogelsgesang added a comment. Add missing dot to comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112648/new/

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

2021-12-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @aaron.ballman can you please commit this on my behalf? I don't have commit access Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112648/new/ https://reviews.llvm.org/D112648

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

2021-12-08 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. > What name and email address would you like me to use for patch attribution? Adrian Vogelsgesang, avogelsges...@salesforce.com Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112648/new/

[PATCH] D113943: Add `withIntrospection` matcher.

2021-12-09 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Nit: Personally, I would prefer the name `withDebugOutput` over `withIntrospection`, as it more clearly describes the intent of this matcher. At least to me, "introspection" is usually something done programatically, i.e. the matcher somehow reflects about its own

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-12-13 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. gentle ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-10 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Happy new year! - and a gentle ping ;) Afaict, there are only two smaller issues remaining (see non-closed inline comments): - do we want a test expectation to check for unmodified code - should we remove a couple of comments from the code Personally, I have no

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-17 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @aaron.ballman @xazax.hun Could I ask one of you to finish the review here? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

2022-02-11 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:508 +int ParamIndex = paramIndex(*FTD, *Auto.getDecl()); +if (ParamIndex < 0) + return true; out of curiosity: when would this happen? Repository: rG LLVM Github

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

2022-02-11 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. code looks good to me, but I don't know the code base well enough for a "LGTM" Thanks for implementing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119537/new/ https://reviews.llvm.org/D119537

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

2022-02-11 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Should this be added to the Release Notes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119537/new/ https://reviews.llvm.org/D119537 ___ cfe-commits mailing list

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2022-02-21 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. > Will you still work on this? I don't know. I currently have more high-priority work, and don't know whether/when I will find time for this review again. Also, I realized that I will have to read up more about class-template parameters, e.g. I don't quite

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-04-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang accepted this revision. avogelsgesang added a comment. This revision is now accepted and ready to land. LGTM (for whatever that's worth - still pretty unexperienced with the clang AST) Thanks for picking this up again! Looking forward to have this available in clangd! :)

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-04-01 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. > Not easy to do in this patch, we don't have a good way to associate fixes > with clang diagnostics yet. Is this a clangd-specific implementation issue or a general short-coming in the LSP protocol? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D122102: [clangd] Introduce "add subclass" tweak

2022-03-20 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added subscribers: usaxena95, kadircet, arphaman, mgorny. Herald added a project: All. avogelsgesang requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This commit

[PATCH] D122101: [clangd] Simplify `insertDecl` and support insertion after the last declaration in a file

2022-03-20 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. avogelsgesang requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. So far

[PATCH] D122102: [clangd] Introduce "add subclass" tweak

2022-03-20 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added reviewers: sammccall, nridge, njames93, hokein. avogelsgesang added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122102/new/ https://reviews.llvm.org/D122102 ___

[PATCH] D122102: [clangd] Introduce "add subclass" tweak

2022-03-22 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Thank for your comments, @adamcz and @njames93! I agree that a "Implement interface" tweak is probably more useful, and I can imagine to evolve this tweak in that direction. The main reason, I first went for the "create a new class from scratch" approach, was

[PATCH] D122102: [clangd] Introduce "add subclass" tweak

2022-03-22 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @adamcz > Ideally I'd like to see this code action show up as code completion option as > well. I think in the long-term such a "Implement methods" code action should also apply as a quickfix for class Base { virtual void foo() = 0; }; class

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-02-10 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Can we add this tweak as the default "fix-it" action for > Class '...' does not declare any constructor to initialize its non-modifiable > me on structs like struct X { int& a; } ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-01-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:26-28 +// struct S { +// S(int x, unique_ptr y) : x(x), y(std::move(y)) {} +// }; ``` // e.g. given `struct S{ int x; unique_ptr y; };`,

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 400820. avogelsgesang added a comment. Still warn for issues in macros even if not offering a fix-it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646 Files:

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @xazax.hun Can you please merge this to `main` on my behalf? (I don't have commit rights) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 400813. avogelsgesang marked an inline comment as done. avogelsgesang added a comment. Properly support macros as requested by @xazax.hun Added test cases for +#define COUNT_ONES(SET) SET.count(1) + // CHECK-FIXES: #define COUNT_ONES(SET) SET.count(1)

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-01-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:26-28 +// struct S { +// S(int x, unique_ptr y) : x(x), y(std::move(y)) {} +// }; njames93 wrote: > avogelsgesang wrote: > > ``` > > //

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-01-18 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49 + Class = N->ASTNode.get(); +if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion()) + return false; njames93

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-01-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:38 + std::string title() const override { +return llvm::formatv("define constructor"); + } `Define constructor` (first letter uppercase) At

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-22 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. @xazax.hun ping re merging this to `main`. Would be amazing to get this in still in time for clang 14. In case it makes merging easier for you, there is a copy of this commit on

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Thanks, @xazax.hun and @whisperity for helping me to get this over the finish line! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112646/new/ https://reviews.llvm.org/D112646

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

2022-07-09 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang/www/cxx_status.html:1183 https://wg21.link/p1874r1;>P1874R1 -Partial +Clang 15 should this be `class="unreleased"` instead of `class="full"`? At least this is what other

[PATCH] D130678: [clang-tools-extra] Refactor to reduce code duplication

2022-07-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang accepted this revision. avogelsgesang added a comment. This revision is now accepted and ready to land. LGTM (note that I am rather inexperienced with LLVM, though, so take my approval with a grain of salt) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-24 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG91389000abe8: [LLDB] Add data formatter for std::coroutine_handle (authored by avogelsgesang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 454830. avogelsgesang added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D132451: [docs] Add examples for printing asynchronous stack for coroutines

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang accepted this revision. avogelsgesang added a comment. This revision is now accepted and ready to land. LGTM, thanks! I guess I will update this documentation with LLDB examples, as soon as https://reviews.llvm.org/D132415 and a couple of follow-up improvements landed. As soon as

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 454911. avogelsgesang marked 6 inline comments as done. avogelsgesang added a comment. addressed comments from @aprantl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang marked an inline comment as done. avogelsgesang added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:44 + +lldbutil.run_to_source_breakpoint(self, '// Break after

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:15 + +class TestCoroutineHandle(TestBase): +def do_test(self, stdlib_type): jingham wrote: >

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 455022. avogelsgesang marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-22 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:1139 + AddCXXSynthetic( // XXX + cpp_category_sp, I need to remove this. This is a left-over from an earlier implementation sketch Repository:

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-22 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. avogelsgesang added reviewers: ChuanqiXu, aprantl, labath, mib, JDevlieghere. Herald added a subscriber: mgorny. Herald added a project: All. avogelsgesang requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers:

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-24 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 455111. avogelsgesang marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-24 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:39 + CompilerType coro_func_type = ast_ctx.CreateFunctionType( + /*result_type*/ void_type, /*args*/ _type, /*num_args*/ 1, + /*is_variadic*/ false, /*qualifiers*/

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

2022-08-23 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 454728. avogelsgesang added a comment. remove unrelated changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132415/new/ https://reviews.llvm.org/D132415 Files:

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

2022-09-20 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang/docs/StandardCPlusPlusModules.rst:916 +We call this manner as two-phase compilation +(sources to BMIs and BMIs to object files) to disambiguate the tranditional +compilation process (sources to object files directly),

[PATCH] D131938: [C++20] [Coroutines] Disable to take the address of labels in coroutines

2022-08-16 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. looks good to me on a high-level, but I don't know clang well enough to confidently approve this change Comment at: clang/lib/Sema/SemaCoroutine.cpp:1107 + + // Corotuines will get splitted into pieces. The GNU address of label + // extension

[PATCH] D131046: [NFC][clang-tools-extra]Replace find/insert with try_emplace

2022-08-03 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang accepted this revision. avogelsgesang added a comment. This revision is now accepted and ready to land. The change itself looks good. Please look into the CI failures before landing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131046: [clang-tools-extra]Replace find/insert with try_emplace

2022-08-03 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added inline comments. Comment at: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:140 +// This replacement is a duplicate of one suggested by another TU. +if (!InsertPos.second) { return;

[PATCH] D131938: [C++20] [Coroutines] Disable to take the address of labels in coroutines

2022-11-14 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment. Personally, I am fine with this change as in review right now. @ychen are you still planning to look further into this issue as part of your alternative patch https://reviews.llvm.org/D132084 ? Otherwise, I would propose that we merge @ChuanqiXu 's change for the

[PATCH] D157246: [clang-tidy] Update tests to include C++23 and C++26

2023-08-06 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang created this revision. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a project: All. avogelsgesang requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This commit changes the

[PATCH] D157246: [clang-tidy] Update tests to include C++23 and C++26

2023-08-07 Thread Adrian Vogelsgesang 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 rGfda777849b00: [clang-tidy] Update tests to include C++23 and C++26 (authored by avogelsgesang). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D157246: [clang-tidy] Update tests to include C++23 and C++26

2023-08-07 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang updated this revision to Diff 547931. avogelsgesang added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157246/new/ https://reviews.llvm.org/D157246 Files: