majnemer added inline comments.
> PrintPreprocessedOutput.cpp:101
> PrintPPOutputPPCallbacks(Preprocessor &pp, raw_ostream &os, bool
> lineMarkers,
> - bool defines, bool UseLineDirectives)
> + bool defines, bool dumpIncludeDirectives,
> + bool UseLineDirectives)
Please use the LLVM naming convention for dumpIncludeDirectives.
> PrintPreprocessedOutput.cpp:399
> + if (DumpIncludeDirectives) {
> + const auto TokenText = PP.getSpelling(IncludeTok);
> + assert(TokenText.size() > 0);
I'd spell out the type of TokenText, it is not clear which overload of
getSpelling will get called.
> PrintPreprocessedOutput.cpp:400
> + const auto TokenText = PP.getSpelling(IncludeTok);
> + assert(TokenText.size() > 0);
> + OS << "#" << TokenText << " "
`assert(!TokenText.empty());`
> PrintPreprocessedOutput.cpp:404
> + << " /* clang -E -dI:";
> + if (SearchPath.size() > 0) {
> + // Print out info about the search path within this comment. We need
!SearchPath.empty()
> PrintPreprocessedOutput.cpp:409-411
> + std::vector<char> Buffer((SearchPath.size() * 2) + 1);
> + auto *BufferChars = &Buffer[0];
> + sanitizePath(BufferChars, Buffer.size(), SearchPath);
Why not have sanitizePath return a `std::vector<char>`?
https://reviews.llvm.org/D25153
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
