sammccall accepted this revision.
sammccall added inline comments.

Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:49
+/// A half-open range inside a particular file, the start offset is included 
+/// the end offset is excluded from the range.
nit: character range (just to be totally explicit)?

Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:115
+  /// expansion.
+  llvm::Optional<FileRange> range(const SourceManager &SM) const;
I think this might need a more explicit name. It's reasonably obvious that this 
needs to be optional for some cases (token pasting), but it's not obvious at 
callsites that (or why) we don't use the spelling location for macro-expanded 

One option would be just do that and add an expandedFromMacro() helper so the 
no-macros version is easy to express too.
If we can't do that, maybe `directlySpelledRange` or something?

Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:121
+  /// are from different files or \p Last is located before \p First.
+  static llvm::Optional<FileRange> range(const SourceManager &SM,
+                                         const syntax::Token &First,
similar to above, I'd naively expect this to return a valid range, given the 
tokens expanded from `assert(X && [[Y.z()]] )`

  rG LLVM Github Monorepo


cfe-commits mailing list

Reply via email to