Hi Daniel, Please always add cfe-commits to CC for all code reviews touching cfe or clang-tools-extra repositories.
In http://reviews.llvm.org/D9079#157602, @danielmarjamaki wrote: > what is your opinion on this refactoring? Seems to make things better. A couple of comments inline. Thanks! ================ Comment at: include/clang/Lex/MacroInfo.h:244 @@ -243,2 +243,3 @@ bool tokens_empty() const { return ReplacementTokens.empty(); } + const SmallVector<Token, 8> &tokens() const { return ReplacementTokens; } ---------------- This should return `const SmallVectorImpl<Token> &` to avoid dependency on the SmallVector inline storage size. ================ Comment at: lib/Frontend/PrintPreprocessedOutput.cpp:67 @@ -66,4 +66,3 @@ SmallString<128> SpellingBuffer; - for (MacroInfo::tokens_iterator I = MI.tokens_begin(), E = MI.tokens_end(); - I != E; ++I) { - if (I->hasLeadingSpace()) + for (const clang::Token &T : MI.tokens()) { + if (T.hasLeadingSpace()) ---------------- There's no need in the `clang::` qualifier here due to the using directive above. http://reviews.llvm.org/D9079 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
