sammccall added a comment.
Sorry, I'm having trouble understanding this patch. Can you try to find some
clearer names for the new concepts, or describe how they differ?
================
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>;
----------------
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.
================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:326
Preprocessor &PP;
+ Callbacks *CB;
};
----------------
Give the class and member more descriptive names?
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:256
+class TokenCollector::Callbacks : public PPCallbacks {
+public:
----------------
what is this class for, what does it do?
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:278
+private:
+ TokenCollector *C;
+ /// Used to detect recursive macro expansions.
----------------
members should have real names
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62953/new/
https://reviews.llvm.org/D62953
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
