sammccall marked 2 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:50
+      S.StartOffset = SM.getFileOffset(Tok.location());
+      S.EndOffset = S.StartOffset + Tok.length();
+      if (S.StartOffset >= SelBegin && S.EndOffset <= SelEnd)
----------------
SureYeaah wrote:
> Would this work correctly for nested templates? Or do we need to use the 
> specialized token length function that we use for toHalfOpenFileRange?
This uses the token list as an intermediary for matching selected chars with 
AST nodes. TokenBuffer will indeed by default lex `>>` as a right shift. So 
we're buggy here, but I think it *mostly* doesn't matter.

If it's a double template:
 - the innermost template will claim it first, if the template range touches 
the selection. Problem: if only the first > is selected, the inner template 
will only be partially selected.
 - the outermost template will not get to claim it at all (if the inner 
template range touches the selection). Problem: if this is the *only* part of 
the outer template that's selected, it will be marked unselected. (This should 
be rare)
 - If the inner template doesn't touch the selection, then the outer template 
will be selected but only partially, which is actually correct.

Examples:
```
a<b<c>>
     ~  b=partial (correct)
  ~~~~  b=partial (incorrect: b=complete) <-- this is the worst case
~~~~~~~ a=complete,b=complete (correct)
      ~ a=partial (correct)
     ~~ b=partial (incorrect: a=partial,b=partial)
```

I'll send a followup to fix this case tomorrow (I think we can just always 
split the token in half) but I don't think it's critical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65486



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to