[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev marked an inline comment as done.
kbobyrev added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:320
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+                          const syntax::TokenBuffer &Tokens);
----------------
sammccall wrote:
> kbobyrev wrote:
> > Maybe `llvm::Optional<>` here?
> Pointers are the idiomatic way to express "optional, by-reference" in LLVM.
> 
> By contrast, optional<Token&> is illegal, and optional<token*> is unidiomatic
Ah, I see, I thought a copy is quite cheap in this case. Makes sense then.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:254
+                              const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef<syntax::Token> All =
----------------
sammccall wrote:
> kbobyrev wrote:
> > Nit: maybe mention this requirement in the header comment?
> It is mentioned - Loc must be a spelling location.
Ah, okay, I didn't know that spelling location implies exactly this property, my bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356