Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -fkeep-system-includes modifier for -E #67684

Closed
wants to merge 6 commits into from

Conversation

pogo59
Copy link
Collaborator

@pogo59 pogo59 commented Sep 28, 2023

This option will cause -E to preserve the #include directives
for system headers, rather than expanding them into the output.
This can greatly reduce the volume of preprocessed source text
in a test case, making test case reduction simpler.

Note that -fkeep-system-includes is not always appropriate. For
example, if the problem you want to reproduce is induced by a
system header file, it's better to expand those headers fully.
If your source defines symbols that influence the content of a
system header (e.g., _POSIX_SOURCE) then -E will eliminate the
definition, potentially changing the meaning of the preprocessed
source. If you use -isystem to point to non-system headers, for
example to suppress warnings in third-party software, those will
not be expanded and might make the preprocessed source less useful
as a test case.

This will allow the raw_ostream to be redirected in a subsequent commit.
This option will cause -E to preserve the #include directives
for system headers, rather than expanding them into the output.
This can greatly reduce the volume of preprocessed source text
in a test case, making test case reduction simpler.

Note that -fkeep-system-includes is not always appropriate. For
example, if the problem you want to reproduce is induced by a
system header file, it's better to expand those headers fully.
If your source defines symbols that influence the content of a
system header (e.g., _POSIX_SOURCE) then -E will eliminate the
definition, potentially changing the meaning of the preprocessed
source. If you use -isystem to point to non-system headers, for
example to suppress warnings in third-party software, those will
not be expanded and might make the preprocessed source less useful
as a test case.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 28, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 28, 2023

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Changes

This option will cause -E to preserve the #include directives
for system headers, rather than expanding them into the output.
This can greatly reduce the volume of preprocessed source text
in a test case, making test case reduction simpler.

Note that -fkeep-system-includes is not always appropriate. For
example, if the problem you want to reproduce is induced by a
system header file, it's better to expand those headers fully.
If your source defines symbols that influence the content of a
system header (e.g., _POSIX_SOURCE) then -E will eliminate the
definition, potentially changing the meaning of the preprocessed
source. If you use -isystem to point to non-system headers, for
example to suppress warnings in third-party software, those will
not be expanded and might make the preprocessed source less useful
as a test case.


Patch is 30.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67684.diff

10 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+2-2)
  • (modified) clang/include/clang/Driver/Options.td (+7)
  • (modified) clang/include/clang/Frontend/PreprocessorOutputOptions.h (+2)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+16-2)
  • (modified) clang/lib/Frontend/PrintPreprocessedOutput.cpp (+123-110)
  • (added) clang/test/Frontend/Inputs/dashE/dashE.h (+3)
  • (added) clang/test/Frontend/Inputs/dashE/sys/a.h (+3)
  • (added) clang/test/Frontend/Inputs/dashE/sys/b.h (+1)
  • (added) clang/test/Frontend/dashE-sysincludes.cpp (+23)
  • (modified) clang/test/Preprocessor/minimize-whitespace-messages.c (+9-6)
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 2a48c063e243ee0..446a610f56f0379 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -172,8 +172,8 @@ def err_drv_invalid_Xopenmp_target_with_args : Error<
   "invalid -Xopenmp-target argument: '%0', options requiring arguments are unsupported">;
 def err_drv_argument_only_allowed_with : Error<
   "invalid argument '%0' only allowed with '%1'">;
-def err_drv_minws_unsupported_input_type : Error<
-  "'-fminimize-whitespace' invalid for input of type %0">;
+def err_drv_opt_unsupported_input_type : Error<
+  "'%0' invalid for input of type %1">;
 def err_drv_amdgpu_ieee_without_no_honor_nans : Error<
   "invalid argument '-mno-amdgpu-ieee' only allowed with relaxed NaN handling">;
 def err_drv_argument_not_allowed_with : Error<
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index f8143651cd3c151..4a64aee64d3f06e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2483,6 +2483,13 @@ defm minimize_whitespace : BoolFOption<"minimize-whitespace",
           "whitespace such that two files with only formatting changes are "
           "equal.\n\nOnly valid with -E on C-like inputs and incompatible "
           "with -traditional-cpp.">, NegFlag<SetFalse>>;
+defm keep_system_includes : BoolFOption<"keep-system-includes",
+  PreprocessorOutputOpts<"KeepSystemIncludes">, DefaultFalse,
+  PosFlag<SetTrue, [], [ClangOption, CC1Option],
+          "Instead of expanding system headers when emitting preprocessor "
+          "output, preserve the #include directive. Useful when producing "
+          "preprocessed output for test case reduction.\n\nOnly valid with -E.">,
+  NegFlag<SetFalse>>;
 
 def ffreestanding : Flag<["-"], "ffreestanding">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
diff --git a/clang/include/clang/Frontend/PreprocessorOutputOptions.h b/clang/include/clang/Frontend/PreprocessorOutputOptions.h
index d542032431b14c8..db2ec9f2ae20698 100644
--- a/clang/include/clang/Frontend/PreprocessorOutputOptions.h
+++ b/clang/include/clang/Frontend/PreprocessorOutputOptions.h
@@ -26,6 +26,7 @@ class PreprocessorOutputOptions {
   unsigned RewriteImports  : 1;    ///< Include contents of transitively-imported modules.
   unsigned MinimizeWhitespace : 1; ///< Ignore whitespace from input.
   unsigned DirectivesOnly : 1; ///< Process directives but do not expand macros.
+  unsigned KeepSystemIncludes : 1; ///< Do not expand system headers.
 
 public:
   PreprocessorOutputOptions() {
@@ -40,6 +41,7 @@ class PreprocessorOutputOptions {
     RewriteImports = 0;
     MinimizeWhitespace = 0;
     DirectivesOnly = 0;
+    KeepSystemIncludes = 0;
   }
 };
 
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index dda6aef641904aa..1101add97b65e85 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -68,7 +68,9 @@ using namespace llvm::opt;
 static void CheckPreprocessingOptions(const Driver &D, const ArgList &Args) {
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_C, options::OPT_CC,
                                options::OPT_fminimize_whitespace,
-                               options::OPT_fno_minimize_whitespace)) {
+                               options::OPT_fno_minimize_whitespace,
+                               options::OPT_fkeep_system_includes,
+                               options::OPT_fno_keep_system_includes)) {
     if (!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_P) &&
         !Args.hasArg(options::OPT__SLASH_EP) && !D.CCCIsCPP()) {
       D.Diag(clang::diag::err_drv_argument_only_allowed_with)
@@ -6713,11 +6715,23 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                    options::OPT_fno_minimize_whitespace, false)) {
     types::ID InputType = Inputs[0].getType();
     if (!isDerivedFromC(InputType))
-      D.Diag(diag::err_drv_minws_unsupported_input_type)
+      D.Diag(diag::err_drv_opt_unsupported_input_type)
+          << "-fminimize-whitespace"
           << types::getTypeName(InputType);
     CmdArgs.push_back("-fminimize-whitespace");
   }
 
+  // -fno-keep-system-includes is default.
+  if (Args.hasFlag(options::OPT_fkeep_system_includes,
+                   options::OPT_fno_keep_system_includes, false)) {
+    types::ID InputType = Inputs[0].getType();
+    if (!isDerivedFromC(InputType))
+      D.Diag(diag::err_drv_opt_unsupported_input_type)
+      << "-fkeep-system-includes"
+      << types::getTypeName(InputType);
+    CmdArgs.push_back("-fkeep-system-includes");
+  }
+
   // -fms-extensions=0 is default.
   if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
                    IsWindowsMSVC))
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index 1b262d9e6f7cb3b..4056649e85c28a9 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -32,42 +32,42 @@ using namespace clang;
 /// PrintMacroDefinition - Print a macro definition in a form that will be
 /// properly accepted back as a definition.
 static void PrintMacroDefinition(const IdentifierInfo &II, const MacroInfo &MI,
-                                 Preprocessor &PP, raw_ostream &OS) {
-  OS << "#define " << II.getName();
+                                 Preprocessor &PP, raw_ostream *OS) {
+  *OS << "#define " << II.getName();
 
   if (MI.isFunctionLike()) {
-    OS << '(';
+    *OS << '(';
     if (!MI.param_empty()) {
       MacroInfo::param_iterator AI = MI.param_begin(), E = MI.param_end();
       for (; AI+1 != E; ++AI) {
-        OS << (*AI)->getName();
-        OS << ',';
+        *OS << (*AI)->getName();
+        *OS << ',';
       }
 
       // Last argument.
       if ((*AI)->getName() == "__VA_ARGS__")
-        OS << "...";
+        *OS << "...";
       else
-        OS << (*AI)->getName();
+        *OS << (*AI)->getName();
     }
 
     if (MI.isGNUVarargs())
-      OS << "...";  // #define foo(x...)
+      *OS << "...";  // #define foo(x...)
 
-    OS << ')';
+    *OS << ')';
   }
 
   // GCC always emits a space, even if the macro body is empty.  However, do not
   // want to emit two spaces if the first token has a leading space.
   if (MI.tokens_empty() || !MI.tokens_begin()->hasLeadingSpace())
-    OS << ' ';
+    *OS << ' ';
 
   SmallString<128> SpellingBuffer;
   for (const auto &T : MI.tokens()) {
     if (T.hasLeadingSpace())
-      OS << ' ';
+      *OS << ' ';
 
-    OS << PP.getSpelling(T, SpellingBuffer);
+    *OS << PP.getSpelling(T, SpellingBuffer);
   }
 }
 
@@ -81,7 +81,7 @@ class PrintPPOutputPPCallbacks : public PPCallbacks {
   SourceManager &SM;
   TokenConcatenation ConcatInfo;
 public:
-  raw_ostream &OS;
+  raw_ostream *OS;
 private:
   unsigned CurLine;
 
@@ -97,20 +97,24 @@ class PrintPPOutputPPCallbacks : public PPCallbacks {
   bool IsFirstFileEntered;
   bool MinimizeWhitespace;
   bool DirectivesOnly;
+  bool KeepSystemIncludes;
+  raw_ostream *OrigOS;
+  std::unique_ptr<llvm::raw_null_ostream> NullOS;
 
   Token PrevTok;
   Token PrevPrevTok;
 
 public:
-  PrintPPOutputPPCallbacks(Preprocessor &pp, raw_ostream &os, bool lineMarkers,
+  PrintPPOutputPPCallbacks(Preprocessor &pp, raw_ostream *os, bool lineMarkers,
                            bool defines, bool DumpIncludeDirectives,
                            bool UseLineDirectives, bool MinimizeWhitespace,
-                           bool DirectivesOnly)
+                           bool DirectivesOnly, bool KeepSystemIncludes)
       : PP(pp), SM(PP.getSourceManager()), ConcatInfo(PP), OS(os),
         DisableLineMarkers(lineMarkers), DumpDefines(defines),
         DumpIncludeDirectives(DumpIncludeDirectives),
         UseLineDirectives(UseLineDirectives),
-        MinimizeWhitespace(MinimizeWhitespace), DirectivesOnly(DirectivesOnly) {
+        MinimizeWhitespace(MinimizeWhitespace), DirectivesOnly(DirectivesOnly),
+        KeepSystemIncludes(KeepSystemIncludes), OrigOS(os) {
     CurLine = 0;
     CurFilename += "<uninit>";
     EmittedTokensOnThisLine = false;
@@ -118,6 +122,8 @@ class PrintPPOutputPPCallbacks : public PPCallbacks {
     FileType = SrcMgr::C_User;
     Initialized = false;
     IsFirstFileEntered = false;
+    if (KeepSystemIncludes)
+      NullOS = std::make_unique<llvm::raw_null_ostream>();
 
     PrevTok.startToken();
     PrevPrevTok.startToken();
@@ -235,23 +241,23 @@ void PrintPPOutputPPCallbacks::WriteLineInfo(unsigned LineNo,
 
   // Emit #line directives or GNU line markers depending on what mode we're in.
   if (UseLineDirectives) {
-    OS << "#line" << ' ' << LineNo << ' ' << '"';
-    OS.write_escaped(CurFilename);
-    OS << '"';
+    *OS << "#line" << ' ' << LineNo << ' ' << '"';
+    OS->write_escaped(CurFilename);
+    *OS << '"';
   } else {
-    OS << '#' << ' ' << LineNo << ' ' << '"';
-    OS.write_escaped(CurFilename);
-    OS << '"';
+    *OS << '#' << ' ' << LineNo << ' ' << '"';
+    OS->write_escaped(CurFilename);
+    *OS << '"';
 
     if (ExtraLen)
-      OS.write(Extra, ExtraLen);
+      OS->write(Extra, ExtraLen);
 
     if (FileType == SrcMgr::C_System)
-      OS.write(" 3", 2);
+      OS->write(" 3", 2);
     else if (FileType == SrcMgr::C_ExternCSystem)
-      OS.write(" 3 4", 4);
+      OS->write(" 3 4", 4);
   }
-  OS << '\n';
+  *OS << '\n';
 }
 
 /// MoveToLine - Move the output to the source line specified by the location
@@ -266,7 +272,7 @@ bool PrintPPOutputPPCallbacks::MoveToLine(unsigned LineNo,
   bool StartedNewLine = false;
   if ((RequireStartOfLine && EmittedTokensOnThisLine) ||
       EmittedDirectiveOnThisLine) {
-    OS << '\n';
+    *OS << '\n';
     StartedNewLine = true;
     CurLine += 1;
     EmittedTokensOnThisLine = false;
@@ -283,12 +289,12 @@ bool PrintPPOutputPPCallbacks::MoveToLine(unsigned LineNo,
     // Printing a single line has priority over printing a #line directive, even
     // when minimizing whitespace which otherwise would print #line directives
     // for every single line.
-    OS << '\n';
+    *OS << '\n';
     StartedNewLine = true;
   } else if (!DisableLineMarkers) {
     if (LineNo - CurLine <= 8) {
       const char *NewLines = "\n\n\n\n\n\n\n\n";
-      OS.write(NewLines, LineNo - CurLine);
+      OS->write(NewLines, LineNo - CurLine);
     } else {
       // Emit a #line or line marker.
       WriteLineInfo(LineNo, nullptr, 0);
@@ -297,7 +303,7 @@ bool PrintPPOutputPPCallbacks::MoveToLine(unsigned LineNo,
   } else if (EmittedTokensOnThisLine) {
     // If we are not on the correct line and don't need to be line-correct,
     // at least ensure we start on a new line.
-    OS << '\n';
+    *OS << '\n';
     StartedNewLine = true;
   }
 
@@ -312,7 +318,7 @@ bool PrintPPOutputPPCallbacks::MoveToLine(unsigned LineNo,
 
 void PrintPPOutputPPCallbacks::startNewLineIfNeeded() {
   if (EmittedTokensOnThisLine || EmittedDirectiveOnThisLine) {
-    OS << '\n';
+    *OS << '\n';
     EmittedTokensOnThisLine = false;
     EmittedDirectiveOnThisLine = false;
   }
@@ -350,6 +356,10 @@ void PrintPPOutputPPCallbacks::FileChanged(SourceLocation Loc,
 
   CurLine = NewLine;
 
+  // In KeepSystemIncludes mode, redirect OS as needed.
+  if (KeepSystemIncludes && (isSystem(FileType) != isSystem(NewFileType)))
+    OS = isSystem(FileType) ? OrigOS : NullOS.get();
+
   CurFilename.clear();
   CurFilename += UserLoc.getFilename();
   FileType = NewFileType;
@@ -394,14 +404,16 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
     StringRef SearchPath, StringRef RelativePath, const Module *Imported,
     SrcMgr::CharacteristicKind FileType) {
   // In -dI mode, dump #include directives prior to dumping their content or
-  // interpretation.
-  if (DumpIncludeDirectives) {
+  // interpretation. Similar for -fkeep-system-includes.
+  if (DumpIncludeDirectives || (KeepSystemIncludes && isSystem(FileType))) {
     MoveToLine(HashLoc, /*RequireStartOfLine=*/true);
     const std::string TokenText = PP.getSpelling(IncludeTok);
     assert(!TokenText.empty());
-    OS << "#" << TokenText << " "
-       << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
-       << " /* clang -E -dI */";
+    *OS << "#" << TokenText << " "
+        << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
+        << " /* clang -E "
+        << (DumpIncludeDirectives ? "-dI" : "-fkeep-system-includes")
+        << " */";
     setEmittedDirectiveOnThisLine();
   }
 
@@ -412,11 +424,12 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
     case tok::pp_import:
     case tok::pp_include_next:
       MoveToLine(HashLoc, /*RequireStartOfLine=*/true);
-      OS << "#pragma clang module import " << Imported->getFullModuleName(true)
-         << " /* clang -E: implicit import for "
-         << "#" << PP.getSpelling(IncludeTok) << " "
-         << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
-         << " */";
+      *OS << "#pragma clang module import "
+          << Imported->getFullModuleName(true)
+          << " /* clang -E: implicit import for "
+          << "#" << PP.getSpelling(IncludeTok) << " "
+          << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
+          << " */";
       setEmittedDirectiveOnThisLine();
       break;
 
@@ -438,14 +451,14 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
 /// Handle entering the scope of a module during a module compilation.
 void PrintPPOutputPPCallbacks::BeginModule(const Module *M) {
   startNewLineIfNeeded();
-  OS << "#pragma clang module begin " << M->getFullModuleName(true);
+  *OS << "#pragma clang module begin " << M->getFullModuleName(true);
   setEmittedDirectiveOnThisLine();
 }
 
 /// Handle leaving the scope of a module during a module compilation.
 void PrintPPOutputPPCallbacks::EndModule(const Module *M) {
   startNewLineIfNeeded();
-  OS << "#pragma clang module end /*" << M->getFullModuleName(true) << "*/";
+  *OS << "#pragma clang module end /*" << M->getFullModuleName(true) << "*/";
   setEmittedDirectiveOnThisLine();
 }
 
@@ -454,8 +467,8 @@ void PrintPPOutputPPCallbacks::EndModule(const Module *M) {
 void PrintPPOutputPPCallbacks::Ident(SourceLocation Loc, StringRef S) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
 
-  OS.write("#ident ", strlen("#ident "));
-  OS.write(S.begin(), S.size());
+  OS->write("#ident ", strlen("#ident "));
+  OS->write(S.begin(), S.size());
   setEmittedTokensOnThisLine();
 }
 
@@ -491,19 +504,19 @@ void PrintPPOutputPPCallbacks::MacroUndefined(const Token &MacroNameTok,
     return;
 
   MoveToLine(MacroNameTok.getLocation(), /*RequireStartOfLine=*/true);
-  OS << "#undef " << MacroNameTok.getIdentifierInfo()->getName();
+  *OS << "#undef " << MacroNameTok.getIdentifierInfo()->getName();
   setEmittedDirectiveOnThisLine();
 }
 
-static void outputPrintable(raw_ostream &OS, StringRef Str) {
+static void outputPrintable(raw_ostream *OS, StringRef Str) {
   for (unsigned char Char : Str) {
     if (isPrintable(Char) && Char != '\\' && Char != '"')
-      OS << (char)Char;
+      *OS << (char)Char;
     else // Output anything hard as an octal escape.
-      OS << '\\'
-         << (char)('0' + ((Char >> 6) & 7))
-         << (char)('0' + ((Char >> 3) & 7))
-         << (char)('0' + ((Char >> 0) & 7));
+      *OS << '\\'
+          << (char)('0' + ((Char >> 6) & 7))
+          << (char)('0' + ((Char >> 3) & 7))
+          << (char)('0' + ((Char >> 0) & 7));
   }
 }
 
@@ -512,25 +525,25 @@ void PrintPPOutputPPCallbacks::PragmaMessage(SourceLocation Loc,
                                              PragmaMessageKind Kind,
                                              StringRef Str) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
-  OS << "#pragma ";
+  *OS << "#pragma ";
   if (!Namespace.empty())
-    OS << Namespace << ' ';
+    *OS << Namespace << ' ';
   switch (Kind) {
     case PMK_Message:
-      OS << "message(\"";
+      *OS << "message(\"";
       break;
     case PMK_Warning:
-      OS << "warning \"";
+      *OS << "warning \"";
       break;
     case PMK_Error:
-      OS << "error \"";
+      *OS << "error \"";
       break;
   }
 
   outputPrintable(OS, Str);
-  OS << '"';
+  *OS << '"';
   if (Kind == PMK_Message)
-    OS << ')';
+    *OS << ')';
   setEmittedDirectiveOnThisLine();
 }
 
@@ -538,8 +551,8 @@ void PrintPPOutputPPCallbacks::PragmaDebug(SourceLocation Loc,
                                            StringRef DebugType) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
 
-  OS << "#pragma clang __debug ";
-  OS << DebugType;
+  *OS << "#pragma clang __debug ";
+  *OS << DebugType;
 
   setEmittedDirectiveOnThisLine();
 }
@@ -547,14 +560,14 @@ void PrintPPOutputPPCallbacks::PragmaDebug(SourceLocation Loc,
 void PrintPPOutputPPCallbacks::
 PragmaDiagnosticPush(SourceLocation Loc, StringRef Namespace) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
-  OS << "#pragma " << Namespace << " diagnostic push";
+  *OS << "#pragma " << Namespace << " diagnostic push";
   setEmittedDirectiveOnThisLine();
 }
 
 void PrintPPOutputPPCallbacks::
 PragmaDiagnosticPop(SourceLocation Loc, StringRef Namespace) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
-  OS << "#pragma " << Namespace << " diagnostic pop";
+  *OS << "#pragma " << Namespace << " diagnostic pop";
   setEmittedDirectiveOnThisLine();
 }
 
@@ -563,25 +576,25 @@ void PrintPPOutputPPCallbacks::PragmaDiagnostic(SourceLocation Loc,
                                                 diag::Severity Map,
                                                 StringRef Str) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
-  OS << "#pragma " << Namespace << " diagnostic ";
+  *OS << "#pragma " << Namespace << " diagnostic ";
   switch (Map) {
   case diag::Severity::Remark:
-    OS << "remark";
+    *OS << "remark";
     break;
   case diag::Severity::Warning:
-    OS << "warning";
+    *OS << "warning";
     break;
   case diag::Severity::Error:
-    OS << "error";
+    *OS << "error";
     break;
   case diag::Severity::Ignored:
-    OS << "ignored";
+    *OS << "ignored";
     break;
   case diag::Severity::Fatal:
-    OS << "fatal";
+    *OS << "fatal";
     break;
   }
-  OS << " \"" << Str << '"';
+  *OS << " \"" << Str << '"';
   setEmittedDirectiveOnThisLine();
 }
 
@@ -590,69 +603,69 @@ void PrintPPOutputPPCallbacks::PragmaWarning(SourceLocation Loc,
                                              ArrayRef<int> Ids) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
 
-  OS << "#pragma warning(";
+  *OS << "#pragma warning(";
   switch(WarningSpec) {
-    case PWS_Default:  OS << "default"; break;
-    case PWS_Disable:  OS << "disable"; break;
-    case PWS_Error:    OS << "error"; break;
-    case PWS_Once:     OS << "once"; break;
-    case PWS_Suppress: OS << "suppress"; break;
-    case PWS_Level1:   OS << '1'; break;
-    case PWS_Level2:   OS << '2'; break;
-    case PWS_Level3:   OS << '3'; break;
-    case PWS_Level4:   OS << '4'; break;
+    case PWS_Default:  *OS << "default"; break;
+    case PWS_Disable:  *OS << "disable"; break;
+    case PWS_Error:    *OS << "error"; break;
+    case PWS_Once:     *OS << "once"; break;
+    case PWS_Suppress: *OS << "suppress"; break;
+    case PWS_Level1:   *OS << '1'; break;
+    case PWS_Level2:   *OS << '2'; break;
+    case PWS_Level3:   *OS << '3'; break;
+    case PWS_Level4:   *OS << '4'; break;
   }
-  OS << ':';
+  *OS << ':';
 
   for (ArrayRef<int>::iterator I = Ids.begin(), E = Ids.end(); I != E; ++I)
-    OS << ' ' << *I;
-  OS << ')';
+    *OS << ' ' << *I;
+  *OS << ')';
   setEmittedDirectiveOnThisLine();
 }
 
 void PrintPPOutputPPCallbacks::PragmaWarningPush(SourceLocation Loc,
                                                  int Level) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
-  OS << "#pragma warning(push";
+  *OS << "#pragma warning(push";
   if (Level >= 0)
-    OS << ", " << Level;
-  OS << ')';
+    *OS << ", " << Level;
+  *OS << ')';
   setEmittedDirectiveOnThisLine();
 }
 
 void PrintPPOutputPPCallbacks::PragmaWarningPop(SourceLocation Loc) {
   MoveToLine(Loc, /*RequireStartOfLine=*/true);
-  OS << "#pragma warning(pop)";
+  *OS << "#pragma warning(pop)";
   setEmi...
[truncated]

@github-actions
Copy link

github-actions bot commented Sep 28, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 4a16b51f438ad2827220bd21799bfee5ff3d55ef 3622e2eff7e9bb6e21336a32fea05fc1b50c21ba -- clang/test/Frontend/Inputs/dashE/dashE.h clang/test/Frontend/Inputs/dashE/sys/a.h clang/test/Frontend/Inputs/dashE/sys/b.h clang/test/Frontend/dashE-sysincludes.cpp clang/include/clang/Frontend/PreprocessorOutputOptions.h clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/PrintPreprocessedOutput.cpp clang/test/Preprocessor/minimize-whitespace-messages.c
View the diff from clang-format here.
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index 7f5f66906823..81ee1654af51 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -52,7 +52,7 @@ static void PrintMacroDefinition(const IdentifierInfo &II, const MacroInfo &MI,
     }
 
     if (MI.isGNUVarargs())
-      *OS << "...";  // #define foo(x...)
+      *OS << "..."; // #define foo(x...)
 
     *OS << ')';
   }
@@ -82,6 +82,7 @@ class PrintPPOutputPPCallbacks : public PPCallbacks {
   TokenConcatenation ConcatInfo;
 public:
   raw_ostream *OS;
+
 private:
   unsigned CurLine;
 
@@ -409,11 +410,9 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
     MoveToLine(HashLoc, /*RequireStartOfLine=*/true);
     const std::string TokenText = PP.getSpelling(IncludeTok);
     assert(!TokenText.empty());
-    *OS << "#" << TokenText << " "
-        << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
-        << " /* clang -E "
-        << (DumpIncludeDirectives ? "-dI" : "-fkeep-system-includes")
-        << " */";
+    *OS << "#" << TokenText << " " << (IsAngled ? '<' : '"') << FileName
+        << (IsAngled ? '>' : '"') << " /* clang -E "
+        << (DumpIncludeDirectives ? "-dI" : "-fkeep-system-includes") << " */";
     setEmittedDirectiveOnThisLine();
   }
 
@@ -424,12 +423,10 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
     case tok::pp_import:
     case tok::pp_include_next:
       MoveToLine(HashLoc, /*RequireStartOfLine=*/true);
-      *OS << "#pragma clang module import "
-          << Imported->getFullModuleName(true)
+      *OS << "#pragma clang module import " << Imported->getFullModuleName(true)
           << " /* clang -E: implicit import for "
-          << "#" << PP.getSpelling(IncludeTok) << " "
-          << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
-          << " */";
+          << "#" << PP.getSpelling(IncludeTok) << " " << (IsAngled ? '<' : '"')
+          << FileName << (IsAngled ? '>' : '"') << " */";
       setEmittedDirectiveOnThisLine();
       break;
 
@@ -513,10 +510,8 @@ static void outputPrintable(raw_ostream *OS, StringRef Str) {
     if (isPrintable(Char) && Char != '\\' && Char != '"')
       *OS << (char)Char;
     else // Output anything hard as an octal escape.
-      *OS << '\\'
-          << (char)('0' + ((Char >> 6) & 7))
-          << (char)('0' + ((Char >> 3) & 7))
-          << (char)('0' + ((Char >> 0) & 7));
+      *OS << '\\' << (char)('0' + ((Char >> 6) & 7))
+          << (char)('0' + ((Char >> 3) & 7)) << (char)('0' + ((Char >> 0) & 7));
   }
 }
 
@@ -605,15 +600,33 @@ void PrintPPOutputPPCallbacks::PragmaWarning(SourceLocation Loc,
 
   *OS << "#pragma warning(";
   switch(WarningSpec) {
-    case PWS_Default:  *OS << "default"; break;
-    case PWS_Disable:  *OS << "disable"; break;
-    case PWS_Error:    *OS << "error"; break;
-    case PWS_Once:     *OS << "once"; break;
-    case PWS_Suppress: *OS << "suppress"; break;
-    case PWS_Level1:   *OS << '1'; break;
-    case PWS_Level2:   *OS << '2'; break;
-    case PWS_Level3:   *OS << '3'; break;
-    case PWS_Level4:   *OS << '4'; break;
+  case PWS_Default:
+    *OS << "default";
+    break;
+  case PWS_Disable:
+    *OS << "disable";
+    break;
+  case PWS_Error:
+    *OS << "error";
+    break;
+  case PWS_Once:
+    *OS << "once";
+    break;
+  case PWS_Suppress:
+    *OS << "suppress";
+    break;
+  case PWS_Level1:
+    *OS << '1';
+    break;
+  case PWS_Level2:
+    *OS << '2';
+    break;
+  case PWS_Level3:
+    *OS << '3';
+    break;
+  case PWS_Level4:
+    *OS << '4';
+    break;
   }
   *OS << ':';
 
@@ -805,7 +818,6 @@ struct UnknownPragmaHandler : public PragmaHandler {
 };
 } // end anonymous namespace
 
-
 static void PrintPreprocessedTokens(Preprocessor &PP, Token &Tok,
                                     PrintPPOutputPPCallbacks *Callbacks) {
   bool DropComments = PP.getLangOpts().TraditionalCPP &&

@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 28, 2023

Its limitations, i.e. how in various cases it can break or change correct code, need to be documented somewhere alongside the option (ideally a disclaimer in --help's output and a longer explanation in the rendered documentation).

@pogo59
Copy link
Collaborator Author

pogo59 commented Sep 29, 2023

@jrtc27 I've beefed up the help text some. I'm unable to find any "rendered documentation" for -E and friends; if you can point me to it, I'd be happy to add the new option there.

@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 29, 2023

@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 29, 2023

@pogo59
Copy link
Collaborator Author

pogo59 commented Sep 29, 2023

Aha! Thank you. I should have found that third one in my searches, although it doesn't really describe the options that affect -E mode. The first link does, somewhat, and seems like the right place to add this new option. I'll have a go at that.

@pogo59
Copy link
Collaborator Author

pogo59 commented Sep 29, 2023

The first link does, somewhat, and seems like the right place to add this new option. I'll have a go at that.

Well, that one appears to be generated from the Options.td file, and simply includes the help text, so in effect I've already done that. :)

UsersManual.html doesn't have a preprocessor options section at all, and I think it's outside the scope of this patch to add one.

clang.html (actually CommandGuide/clang.rst) doesn't really talk about the other -E affecting options, but at least it has a subsection about preprocessor options, so I'll see what I can do there.

@pogo59
Copy link
Collaborator Author

pogo59 commented Sep 29, 2023

Also added a release note.

@pogo59
Copy link
Collaborator Author

pogo59 commented Oct 6, 2023

Ping @erichkeane were you willing to approve both this and #67613?

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too fails the formatter. That said, I have no comments, though preprocessing isn't my place of expertise. Most of the changes are pretty mild though, and the approach of just replacing the output file with a null-OS seems appropriate. Fix the formatting, and I'll accept, but please give others (Aaron in particular) time to take a look before committing.

@pogo59
Copy link
Collaborator Author

pogo59 commented Oct 6, 2023

Many of the clang-format complaints on this one are from the first commit, which I wanted to make as minimalist as possible. I believe I didn't make the file any less conformant in that commit. :) I will fix the others.

@pogo59
Copy link
Collaborator Author

pogo59 commented Oct 6, 2023

Something got screwed up when I tried to squash all but the first commit together. I put it back together manually. The NFC commit was pushed as 9500616, the functional stuff squashed and committed as 71d83bb.

@pogo59 pogo59 closed this Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants