aaron.ballman added a comment. Just some minor nits from me, but generally LG.
================ Comment at: clang/include/clang/Lex/MacroInfo.h:243 - using tokens_iterator = SmallVectorImpl<Token>::const_iterator; + using tokens_iterator = const Token *; + ---------------- I think this should be a `const_tokens_iterator` instead (and it's fine that we don't expose a non-const interface for the iterator). ================ Comment at: clang/include/clang/Lex/MacroInfo.h:256 + allocateTokens(unsigned NumTokens, llvm::BumpPtrAllocator &PPAllocator) { + NumReplacementTokens = NumTokens; + Token *NewReplacementTokens = PPAllocator.Allocate<Token>(NumTokens); ---------------- Should we assert that we've not already allocated tokens before? ================ Comment at: clang/lib/Lex/MacroInfo.cpp:33 + +// MacroInfo is expected to take 40 bytes on platforms with an 8 byte pointer. +template <int> class MacroInfoSizeChecker { ---------------- Should we do this dance for 32-bit systems as well? ================ Comment at: clang/lib/Lex/MacroInfo.cpp:59 + auto ReplacementTokens = tokens(); if (ReplacementTokens.empty()) ---------------- Please spell out the type. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117348/new/ https://reviews.llvm.org/D117348 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits