[PATCH] D25153: preprocessor supports `-dI` flag

David Majnemer via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 4 17:57:10 PDT 2016


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





More information about the cfe-commits mailing list