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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:320
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+                          const syntax::TokenBuffer &Tokens);
----------------
Maybe `llvm::Optional<>` here?


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:254
+                              const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef<syntax::Token> All =
----------------
Nit: maybe mention this requirement in the header comment?


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:257
+      Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
+  // SourceLocation < SourceLocation is OK for one file ID.
+  auto *Right = llvm::partition_point(
----------------
Nit: "Comparing SourceLocations within one file using operator less/< is a valid operation"? Can't come up with something better, but this is slightly hard to understand.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:262
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < Loc);
+  return llvm::makeArrayRef(Right - int(AcceptLeft), Right + int(AcceptRight));
+}
----------------
Nit: static casts here or in variable declarations?


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:267
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+                          const syntax::TokenBuffer &Tokens) {
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
----------------
The formatting looks weird here, does it come from Clang-Format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356