[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti added a comment. Thank you. Could you please commit this for me with `Julian Schmidt <44101708+5chmi...@users.noreply.github.com>`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D141757

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti marked an inline comment as done. 5chmidti added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:187 +// Allow expressions, but only allow completely selected lambda +// expressions or unselected lambda

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti updated this revision to Diff 556349. 5chmidti added a comment. Fixup to last revision change: - do not block the partial selection of lambdas There is no reason to block the partial selection of lambdas. Other expressions can be partially selected as well and still offer an

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti updated this revision to Diff 556348. 5chmidti added a comment. addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D141757 Files:

[PATCH] D138499: [clangd] Extract Function: add hoisting support

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

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti marked an inline comment as done. 5chmidti added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D141757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti added a comment. Thanks for your review comments :) . I'll incorporate them and reference your comments when I post the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti added a comment. In D128861#4604193 , @PiotrZSL wrote: > But for me auto-fixes in this check aren't needed, this simply complicate a > check that could be very simple. You might be right. I will fix the issues I have found and will propose

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti abandoned this revision. 5chmidti added a comment. Because there was no activity on this patch, I am closing it. I should've pinged a bit more :) With the move to GitHub pull requests soon, pushing this patch further in phabricator does not make sense to me. Instead, I could reopen

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti added a comment. Ping. Because Phabricator is set to be made read-only on the 1. of October, it might be better to not review this patch here and instead open it on GitHub instead. Thoughts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti marked 4 inline comments as done. 5chmidti added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:91 + +// Local variables declared inside of the selected lambda cannot go out of +// scope. The DeclRefExprs that are

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti updated this revision to Diff 552065. 5chmidti added a comment. Addressed comments: - added explicit template parameters, trailing return-type declarations and attributes to the visitor that finds DeclRefExprs - added a few more comments about why an extraction is expected to be

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti marked an inline comment as done. 5chmidti added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:91 + +// Local variables declared inside of the selected lambda cannot go out of +// scope. The DeclRefExprs that are

[PATCH] D157773: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

5chmidti added a comment. Thanks. I don't have commit access, could somebody please commit this for me with `Julian Schmidt <44101708+5chmi...@users.noreply.github.com>`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157773/new/

[PATCH] D157773: [clang-tidy] fix None tmpdir when exporting fixes in run-clang-tidy

5chmidti created this revision. 5chmidti added a project: clang-tools-extra. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. 5chmidti requested review of this revision. Herald added a subscriber: cfe-commits.

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

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

[PATCH] D138499: [clangd] Extract Function: add hoisting support

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

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

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

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti requested review of this revision. 5chmidti added a comment. Never mind, the problem is unrelated to this patch. Filed clangd issue #1710 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti planned changes to this revision. 5chmidti added a comment. I found a new issue. I'll probably fix it by next weekend. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138499/new/ https://reviews.llvm.org/D138499

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti updated this revision to Diff 538289. 5chmidti added a comment. Ping, and: - removed mention of captures from the condition about blocking extraction because that is allowed - removed `(of a lambda)` from the following comment because it is not just about defaulted parameters of

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti updated this revision to Diff 538286. 5chmidti added a comment. Ping, and: - change find_if of post-use detection to range-for with a filtered range - make `renderHoistedCall` a private member of `NewFunction` - remove the `Render` lambda in `renderHoistSet` in favor of putting the body

[PATCH] D138499: [clangd] Extract Function: add hoisting support

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

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

5chmidti added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:178 + // Allow all expressions except partial LambdaExpr selections since we + // don't want to extract from the captures/default arguments of a lambda + if

[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

5chmidti updated this revision to Diff 518159. 5chmidti added a comment. I took a little break, but here are the changes/fixes: - moved the logic for variables referenced in captures into the visitor - short circuiting the `TraverseLambdaExpression` and using `TraverseLambdaCapture` to handle

[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

5chmidti added a comment. You're right, I'll incorporate the logic for lambdas into `FindDeclRefsVisitor` and change the docs/comments/commit message to reflect this. I will try to think of more edge cases to test as well. Though I'll be busy until mid march with uni, so there will be no new

[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

5chmidti updated this revision to Diff 491319. 5chmidti added a comment. Herald added a subscriber: ChuanqiXu. 1. addressed comments 2 bugfix (sorry, should've said something) Local variables inside the lambda were previously added to the ReferencedDecls vector and would block the action inside

[PATCH] D141760: [clangd] fix extract function tweak signature spelling out lambda types

5chmidti abandoned this revision. 5chmidti added a comment. Nvmd, `Wrapped` won't work. I'll look into this some more. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141760/new/ https://reviews.llvm.org/D141760

[PATCH] D141760: [clangd] fix extract function tweak signature spelling out lambda types

5chmidti created this revision. 5chmidti added reviewers: sammccall, nridge. Herald added subscribers: kadircet, arphaman. Herald added a project: All. 5chmidti requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D138499: [clangd] Extract Function: add hoisting support

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

[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

5chmidti updated this revision to Diff 489232. 5chmidti added a comment. Add change to release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D141757 Files:

[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

5chmidti created this revision. 5chmidti added reviewers: sammccall, nridge. Herald added subscribers: kadircet, arphaman. Herald added a project: All. 5chmidti requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

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

[PATCH] D138499: [clangd] Extract Function: add hoisting support

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

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti updated this revision to Diff 477204. 5chmidti added a comment. Fix windows build by setting the c++ standard for the hoisting tests explicitly to c++17. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138499/new/

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti updated this revision to Diff 477184. 5chmidti added a comment. Fixup: rm added includes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138499/new/ https://reviews.llvm.org/D138499 Files:

[PATCH] D138499: [clangd] Extract Function: add hoisting support

5chmidti created this revision. 5chmidti added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman. Herald added a project: All. 5chmidti requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

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

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

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

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 454292. 5chmidti added a comment. lower c++ version requirement by making the fixes conditional on c++20 instead of the whole check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 454287. 5chmidti added a comment. - address all comments - minor renames of variables/functions - add missing & when accessing OptTokens value - rebase onto current HEAD - fix llvm::Optional member deprecation warnings - add support to match when the

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti added a comment. I've got everything done locally, but I'm waiting on the decision on the bitset to update the diff, including some minor additional updates I will list then. Comment at:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

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

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti added a comment. Gentle Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

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

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti added a comment. Question: Should this check be extended to also include C.86 : "Flag an operator==() for which the argument types differ; same

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 441197. 5chmidti added a comment. Address inlining isLanguageVersionSupported Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 Files:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 441196. 5chmidti added a comment. Fixup for the same reason Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 Files:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 441194. 5chmidti added a comment. Fixup addressing comments, missed changing the links for docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 Files:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 441193. 5chmidti added a comment. Addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 Files:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 441186. 5chmidti added a comment. rm wrong op from isComparisonOperator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 Files:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti updated this revision to Diff 441185. 5chmidti added a comment. rm unused function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128861/new/ https://reviews.llvm.org/D128861 Files:

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

5chmidti created this revision. 5chmidti added reviewers: aaron.ballman, njames93, alexfh, LegalizeAdulthood, Eugene.Zelenko. Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: All. 5chmidti requested review of this revision. Herald