Re: [PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Sam McCall via cfe-commits
Thanks! I suspect shared libraries makes a difference here, because the tweaks use an unusual library type (object library) to force linking. On Fri, Feb 1, 2019 at 3:01 AM Nathan Ridge via Phabricator < revi...@reviews.llvm.org> wrote: > nridge added a comment. > > Fix here: https://reviews.llvm

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Fix here: https://reviews.llvm.org/D57560 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. The complete commands that's failing is: /usr/bin/clang++-8 -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-l

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D56611#1379876 , @sammccall wrote: > Hi Nathan, > What platform is this on? Not seeing it on the buildbots. > Anything unusual in build setup (standalone build, building with shared > libraries, etc)? I'm on Linux, building

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D56611#1379785 , @nridge wrote: > This commit is causing clangd to fail to link, with errors like this: > > > tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/SwapIfBranches.cpp.o:SwapIfBr

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Herald added a project: LLVM. This commit is causing clangd to fail to link, with errors like this: tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/SwapIfBranches.cpp.o:SwapIfBranches.cpp:function clang::RecursiveASTVisitor::Traverse

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352796: [clangd] A code action to swap branches of an if statement (authored by ibiryukov, committed by ). Changed prior to commit: https://reviews.llvm.org/D56611?vs=184588&id=184594#toc Repository:

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 184588. ilya-biryukov marked 8 inline comments as done. ilya-biryukov added a comment. Herald added a project: clang. Herald added a subscriber: llvm-commits. - Remove Dummy.cpp - Add halfOpenRangeTouches - Add a comment about file vs expansion locations

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-31 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.h:63 +/// Turns a token range into a half-open range and checks its correctness. +/// The resulting range will have only valid source location on both sides, both sammccall wr

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/SourceCode.h:63 +/// Turns a token range into a half-open range and checks its correctness. +/// The resulting range will hav

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-29 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 184041. ilya-biryukov added a comment. - Update the license header CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/SourceCode

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/refactor/tweaks/SwapIfBranches.cpp:34 +/// After: +/// if (foo) { continue; } else { return 10; } +class SwapIfBranches : public Tweak { sammccall wrote: > T

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182910. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Fix a typo in a comment: isValidRange -> isValidFileRange - Make action available under 'else' keywords and conditions - Move the logic of creating replacements to a

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182900. ilya-biryukov added a comment. - Use a helper to avoid creating RewriteBuffer CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extr

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182884. ilya-biryukov added a comment. - Use helper to avoid creating RewriteBuffer - Use std::string to fix stack corruption CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-22 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182872. ilya-biryukov added a comment. - Update id of swap branches in tests - Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extr

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182555. ilya-biryukov added a comment. - Add tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/SourceCode.h clang-tool

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182546. ilya-biryukov added a comment. - Move source code helpers to this patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clan

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Cool! Adopting this one as the simplest tweak for "how should tweaks do X" questions. This depends on helpers not (yet) present in this patch, so not commenting on them now. Need unit tests for tweaks. Something like this should work: Annotations Test(R"cpp(void f

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182321. ilya-biryukov added a comment. - Remove the header file Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/refactor/tweaks/CMakeLists.txt cl

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182317. ilya-biryukov added a comment. - Update to reflect changes in parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/refactor/twea

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182274. ilya-biryukov added a comment. - Rebase after parent change Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/refactor/tweaks/CMakeLists.txt

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182102. ilya-biryukov added a comment. - Fix a typo in the id of the SwapIfBranches Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/CMakeLists.txt

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 182080. ilya-biryukov added a comment. - Update after changes to parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/CMakeLists.txt c

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-15 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/refactor/actions/SwapIfBranches.cpp:36 + + bool VisitIfStmt(IfStmt *If) { +auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(), jkorous wrote: > It seems to me we don't find If token

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-14 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment. Hi Ilya, this seems really useful for people learning how to implement their custom actions! Comment at: clangd/refactor/actions/SwapIfBranches.cpp:36 + + bool VisitIfStmt(IfStmt *If) { +auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.get

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-11 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. A deliberately simple syntactic transformation. Missing tests, but should work very reliably. To serve as an reference point for writing similar actions. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https:

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

2019-01-11 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, mgorny. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D56611 Files: clangd/CMakeLists.txt clangd/CodeActions.cpp clangd/re