ilya-biryukov added inline comments.

Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:115
+  /// expansion.
+  llvm::Optional<FileRange> range(const SourceManager &SM) const;
sammccall wrote:
> 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 tokens.
> 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?
As mentioned in the offline conversation, the idea is that mapping from a token 
inside a macro expansion to a spelled token should be handled by `TokenBuffer` 
and these two functions is really just a convenience helper to get to a range 
*after* the mapping.

This change has been boiling for some time and I think the other bits of it 
seem to be non-controversial and agreed upon.
Would it be ok if we land it with this function using a more concrete name 
(`directlySpelledRange` or something else) or move it into a separate change?

There's a follow-up change that adds an 'expand macro' action to clangd, which 
both has a use-site for these method and provides a functional feature based on 
`TokenBuffer`. Iterating on the design with (even a single) use-case should be 

  rG LLVM Github Monorepo


cfe-commits mailing list

Reply via email to