kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
LGTM, thanks! ================ Comment at: clangd/Selection.cpp:45 + if (R.first > Begin) + return false; // [R.First, Begin) is not covered. + if (Begin < R.second) ---------------- I suppose comment should be `[Begin, R.first)`. Could you also change the order within condition? ================ Comment at: clangd/Selection.cpp:48 + Begin = R.second; // Prefix is covered, truncate the range. + if (Begin >= End) + return true; ---------------- nit: maybe move this check into previous condition? i.e: ``` if (Begin < R.second) { Begin = R.second; // Prefix is covered, truncate the range. if (Begin >= End) return true; } ================ Comment at: clangd/Selection.cpp:236 // This runs for every node in the AST, and must be fast in common cases. // This is called from pop(), so we can take children into account. + SelectionTree::Selection claimRange(SourceRange S) { ---------------- also called from push() now ================ Comment at: clangd/unittests/SelectionTests.cpp:170 + struct S { S(const char*); }; + S [[s ^= "foo"]]; + )cpp", ---------------- could you also add a test case with cursor on identifier(i.e, `s`) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63760/new/ https://reviews.llvm.org/D63760 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits