[PATCH] D56610: [clangd] A code action to qualify an unqualified name

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 16 09:52:00 PST 2019


ilya-biryukov added inline comments.


================
Comment at: clangd/SourceCode.h:64
 
+/// Turns a token range into a half-open range and checks its correctness.
+/// The resulting range will have only valid source location on both sides, both
----------------
jkorous wrote:
> It seems to me the range is actually closed on both sides.
> Do I get it right that the result is?
> 
> ```
> [begin of first token : end of last token]
> ```
> 
> Wouldn't some name like `toWholeTokenRange` be easier to understand?
The range is supposed to be half-open to allow representing points as empty range starting at the specified position.
This literally represents two byte offsets inside the same file. 

My plan is to introduce a class to capture this abstraction, but let me see how the review goes.


================
Comment at: clangd/SourceCode.h:81
+/// Preconditions: isValidFileRange(R) == true, L is a file location.
+bool halfOpenRangeContains(const SourceManager &Mgr, SourceRange R,
+                           SourceLocation L);
----------------
jkorous wrote:
> I'd find it helpful to mention that the range is actually interpreted as closed-open (knowing which half is which).
Good point, will do.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610





More information about the cfe-commits mailing list