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

[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 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?
In step (3) the constructed range is intended to be `SourceRange{1:1, 1:3}`, i.e. it should cover both chars of the `if` keyword.
I haven't checked it, though, will make sure that's the case when actually testing this.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56611