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

[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.getLangOpts(),
----------------
It seems to me we don't find If token whenever
CursorLoc == location of 'f' of the If token

For example if there's "if" starting at location 1:1 I think we proceed like this (hope my pseudocode is clear):

1. `If->getIfLoc()` returns `SourceLocation{1:1}`
2. we construct `SourceRange{begin = 1:1, end = 1:1}`
3. `toHalfOpenFileRange()` returns `SourceRange{1:1, 1:2}`
4. the condition for `SourceLocation L` in `halfOpenRangeContains()` is `1:1 <= LOffset && LOffset < 1:2` which is only ever true for L == 1:1

Do I understand it right?


================
Comment at: clangd/refactor/actions/SwapIfBranches.cpp:62
+
+  // avoid dealing with single-statement brances, they require careful handling
+  // to avoid changing semantics of the code (i.e. dangling else).
----------------
Just a typo in comment `s/brances/branches/`


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56611/new/

https://reviews.llvm.org/D56611