[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:204
+  /// An expansion produced by the preprocessor, includes macro expansions and
+  /// macro directives. Preprocessor always maps a non-empty range of spelled
+  /// tokens to a (possibly empty) range of expanded tokens.
----------------
macro directives -> preprocessor directives?


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:207
+  /// Here is a few examples of expansions:
+  ///    #pragme once      // Expansion from "#pragma once" to an empty range.
+  ///    #define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.
----------------
nit: "pragma"


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:207
+  /// Here is a few examples of expansions:
+  ///    #pragme once      // Expansion from "#pragma once" to an empty range.
+  ///    #define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.
----------------
sammccall wrote:
> nit: "pragma"
these would be clearer without repeating the text, e.g. "Expands to an empty range"


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:209
+  ///    #define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.
+  ///    FOO               // Expansion from "FOO" to "1 2 3".
+  struct Expansion {
----------------
also mention #include?


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:214
+  };
+  /// If \p Spelled starts a mapping (e.g. if it's a macro name) return the
+  /// subrange of expanded tokens that the macro expands to.
----------------
maybe mention `#` as it's the other common case


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:216
+  /// subrange of expanded tokens that the macro expands to.
+  llvm::Optional<Expansion> findExpansion(const syntax::Token *Spelled) const;
+
----------------
I think expansionStartingAt would be a clearer name. Given the current name, I would expect to be able to pass any of the tokens within the spelled range


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954