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