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 simpler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59887/new/ https://reviews.llvm.org/D59887 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
