elsteveogrande updated this revision to Diff 73518.
elsteveogrande added a comment.

- Fix a few more style nits according to LLVM style guide.
- Further fixed w/ `clang-format -style=LLVM`, kept (most of) recommended 
changes.
- Added more unit tests (more include cases: `#include_next`, `-imacro`)
- Drop token-to-text function; discovered `PP.getSpelling(token)`.
- Fixed an existing FIXME where we want to preserve the correct inclusion token 
and not use a hardcoded one.
- Clean up escape function, make safer w/r/t assumptions about buffer sizes and 
make parameter passing less ugly.  Improved logic, finally got simple escaping 
of end-of-comment sequence right after several hairbrained tries :)

Thanks for taking time to provide input @rsmith @majnemer !


https://reviews.llvm.org/D25153

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/PreprocessorOutputOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/PrintPreprocessedOutput.cpp
  test/Preprocessor/dump_import.h
  test/Preprocessor/dump_import.m
  test/Preprocessor/dump_include.c
  test/Preprocessor/dump_include.h

Index: test/Preprocessor/dump_include.h
===================================================================
--- /dev/null
+++ test/Preprocessor/dump_include.h
@@ -0,0 +1,2 @@
+#pragma once
+#define DUMP_INCLUDE_TESTVAL 1
Index: test/Preprocessor/dump_include.c
===================================================================
--- /dev/null
+++ test/Preprocessor/dump_include.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -w -E -dI -isystem %S %s -o - | grep '^#include *<dump_'
+// RUN: %clang_cc1 -w -E -dI -isystem %S %s -o - | grep '^#include *"dump_'
+// RUN: %clang_cc1 -w -E -dI -isystem %S %s -o - | grep '^#include_next *"dump_'
+// RUN: %clang_cc1 -w -E -dI -isystem %S -imacros %S/dump_include.h %s -o - | grep '^#__include_macros "/.*/dump_'
+
+// See also `dump_import.m` which tests the `#import` directive with `-dI`.
+
+#include <dump_include.h>
+#include "dump_include.h"
+#include_next "dump_include.h"
Index: test/Preprocessor/dump_import.m
===================================================================
--- /dev/null
+++ test/Preprocessor/dump_import.m
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -E -dI %s -o - | grep '^#import'
+
+// See also `dump_include.c` which tests other inclusion cases with `-dI`.
+
+#import "dump_import.h"
Index: test/Preprocessor/dump_import.h
===================================================================
--- /dev/null
+++ test/Preprocessor/dump_import.h
@@ -0,0 +1 @@
+#define DUMP_IMPORT_TESTVAL 1
Index: lib/Frontend/PrintPreprocessedOutput.cpp
===================================================================
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -93,13 +93,16 @@
   bool Initialized;
   bool DisableLineMarkers;
   bool DumpDefines;
+  bool DumpIncludeDirectives;
   bool UseLineDirectives;
   bool IsFirstFileEntered;
 public:
   PrintPPOutputPPCallbacks(Preprocessor &pp, raw_ostream &os, bool lineMarkers,
-                           bool defines, bool UseLineDirectives)
+                           bool defines, bool dumpIncludeDirectives,
+                           bool UseLineDirectives)
       : PP(pp), SM(PP.getSourceManager()), ConcatInfo(PP), OS(os),
         DisableLineMarkers(lineMarkers), DumpDefines(defines),
+        DumpIncludeDirectives(dumpIncludeDirectives),
         UseLineDirectives(UseLineDirectives) {
     CurLine = 0;
     CurFilename += "<uninit>";
@@ -311,6 +314,51 @@
   }
 }
 
+/// Escape a pathname appropriate for use in preprocessed output, e.g. if using
+/// '-dI' to print info about inclusions of a header within the given \p Path.
+///
+/// This escapes certain characters unsafe for strings within double-quotes, in
+/// turn within a block comment, using a backslash.  Thus the double-quote and
+/// backslash chars are escaped with `\`.  A `*/` (end-of-block-comment)
+/// sequence will become `*\/`.
+///
+/// The escaped (C-style) string is written into \p Buffer which is expected to
+/// have enough space; specify \p BufferSize to ensure we are doing so safely.
+/// This function will abort if insufficient space is provided.
+///
+/// \param [out] Buffer pointer to characters to receive escaped string plus
+///        NUL terminating byte.
+/// \param BufferSize Size of this buffer, in bytes.
+/// \param Path Path name.
+static void sanitizePath(char *Buffer, size_t BufferSize, StringRef Path) {
+  size_t Size = 0;
+  auto Out = [&](char Ch) {
+    assert(Size < BufferSize);
+    Buffer[Size++] = Ch;
+  };
+  const size_t N = Path.size();
+  size_t I = 0;
+  while (I < N) {
+    if (Path[I] == '\\' || Path[I] == '\"') {
+      // Have to escape backslashes or double-quotes.
+      // Send out backslash to escape the next char.
+      Out('\\');
+    } else if (Path[I] == '*' && (I + 1) < N && Path[I + 1] == '/') {
+      // In the exceedingly strange case of the path having a "*/" sequence,
+      // escape only the slash.  Note that we don't always escape slashes, only
+      // slashes as part of this sequence.
+      // Copy out the asterisk and consume it...
+      Out(Path[I++]);
+      // Send out backslash to escape the next char (i.e. the slash).
+      Out('\\');
+    }
+    // Consume and write out the next input char.
+    Out(Path[I++]);
+  }
+  // Write out NUL terminator for C-style string.
+  Out('\0');
+}
+
 void PrintPPOutputPPCallbacks::InclusionDirective(SourceLocation HashLoc,
                                                   const Token &IncludeTok,
                                                   StringRef FileName,
@@ -320,20 +368,20 @@
                                                   StringRef SearchPath,
                                                   StringRef RelativePath,
                                                   const Module *Imported) {
-  // When preprocessing, turn implicit imports into @imports.
-  // FIXME: This is a stop-gap until a more comprehensive "preprocessing with
-  // modules" solution is introduced.
   if (Imported) {
+    // When preprocessing, turn implicit imports into @imports.
+    // FIXME: This is a stop-gap until a more comprehensive "preprocessing with
+    // modules" solution is introduced.
     startNewLineIfNeeded();
     MoveToLine(HashLoc);
     if (PP.getLangOpts().ObjC2) {
       OS << "@import " << Imported->getFullModuleName() << ";"
          << " /* clang -E: implicit import for \"" << File->getName()
          << "\" */";
     } else {
-      // FIXME: Preseve whether this was a
-      // #include/#include_next/#include_macros/#import.
-      OS << "#include "
+      const auto TokenText = PP.getSpelling(IncludeTok);
+      assert(TokenText.size() > 0);
+      OS << "#" << TokenText << " "
          << (IsAngled ? '<' : '"')
          << FileName
          << (IsAngled ? '>' : '"')
@@ -344,6 +392,31 @@
     // line immediately.
     EmittedTokensOnThisLine = true;
     startNewLineIfNeeded();
+  } else {
+    // Not a module import; it's a more vanilla inclusion of some file using one
+    // of: #include, #import, #include_next, #include_macros.
+    if (DumpIncludeDirectives) {
+      const auto TokenText = PP.getSpelling(IncludeTok);
+      assert(TokenText.size() > 0);
+      OS << "#" << TokenText << " "
+         << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
+         << " /* clang -E -dI:";
+      if (SearchPath.size() > 0) {
+        // Print out info about the search path within this comment.  We need
+        // to escape it because we're printing a quoted string within the
+        // comment block.
+        // This buffer provides enough space for the C-style string.
+        std::vector<char> Buffer((SearchPath.size() * 2) + 1);
+        auto *BufferChars = &Buffer[0];
+        sanitizePath(BufferChars, Buffer.size(), SearchPath);
+        OS << " path=\"" << BufferChars << "\"";
+      } else {
+        OS << " absolute";
+      }
+      OS << " */";
+      setEmittedDirectiveOnThisLine();
+      startNewLineIfNeeded();
+    }
   }
 }
 
@@ -751,7 +824,8 @@
   PP.SetCommentRetentionState(Opts.ShowComments, Opts.ShowMacroComments);
 
   PrintPPOutputPPCallbacks *Callbacks = new PrintPPOutputPPCallbacks(
-      PP, *OS, !Opts.ShowLineMarkers, Opts.ShowMacros, Opts.UseLineDirectives);
+      PP, *OS, !Opts.ShowLineMarkers, Opts.ShowMacros,
+      Opts.ShowIncludeDirectives, Opts.UseLineDirectives);
 
   // Expand macros in pragmas with -fms-extensions.  The assumption is that
   // the majority of pragmas in such a file will be Microsoft pragmas.
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2332,6 +2332,7 @@
   Opts.ShowLineMarkers = !Args.hasArg(OPT_P);
   Opts.ShowMacroComments = Args.hasArg(OPT_CC);
   Opts.ShowMacros = Args.hasArg(OPT_dM) || Args.hasArg(OPT_dD);
+  Opts.ShowIncludeDirectives = Args.hasArg(OPT_dI);
   Opts.RewriteIncludes = Args.hasArg(OPT_frewrite_includes);
   Opts.UseLineDirectives = Args.hasArg(OPT_fuse_line_directives);
 }
Index: include/clang/Frontend/PreprocessorOutputOptions.h
===================================================================
--- include/clang/Frontend/PreprocessorOutputOptions.h
+++ include/clang/Frontend/PreprocessorOutputOptions.h
@@ -22,6 +22,7 @@
   unsigned UseLineDirectives : 1;   ///< Use \#line instead of GCC-style \# N.
   unsigned ShowMacroComments : 1;  ///< Show comments, even in macros.
   unsigned ShowMacros : 1;         ///< Print macro definitions.
+  unsigned ShowIncludeDirectives : 1;  ///< Print includes, imports etc. within preprocessed output.
   unsigned RewriteIncludes : 1;    ///< Preprocess include directives only.
 
 public:
@@ -32,6 +33,7 @@
     UseLineDirectives = 0;
     ShowMacroComments = 0;
     ShowMacros = 0;
+    ShowIncludeDirectives = 0;
     RewriteIncludes = 0;
   }
 };
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -429,6 +429,8 @@
 def dA : Flag<["-"], "dA">, Group<d_Group>;
 def dD : Flag<["-"], "dD">, Group<d_Group>, Flags<[CC1Option]>,
   HelpText<"Print macro definitions in -E mode in addition to normal output">;
+def dI : Flag<["-"], "dI">, Group<d_Group>, Flags<[CC1Option]>,
+  HelpText<"Print include directives in -E mode in addition to normal output">;
 def dM : Flag<["-"], "dM">, Group<d_Group>, Flags<[CC1Option]>,
   HelpText<"Print macro definitions in -E mode instead of normal output">;
 def dead__strip : Flag<["-"], "dead_strip">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to