[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 19 03:40:44 PDT 2019


ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:318
+  ///   2. macro name and arguments for macro expansions.
+  using SpelledMappings =
+      llvm::DenseMap</*SourceLocation*/ int, SourceLocation>;
----------------
sammccall wrote:
> There are now 4 things called mappings, and I can't understand how they relate to each other. I think this needs new names and/or concepts.
Renamed to `PPExpansions`.


================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:326
   Preprocessor &PP;
+  Callbacks *CB;
 };
----------------
sammccall wrote:
> Give the class and member more descriptive names?
Renamed to `Expansions`


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:256
 
+class TokenCollector::Callbacks : public PPCallbacks {
+public:
----------------
sammccall wrote:
> what is this class for, what does it do?
Added a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62953





More information about the cfe-commits mailing list