[clang][driver] Suppress gnu-line-marker when saving temps

[macurtis-amd]

When passing -save-temps to clang, the generated preprocessed output uses gnu line markers. This unexpectedly triggers gnu-line-marker warnings when used with -Weverything or -pedantic. Even worse, compilation fails if -Werror is used.

This change suppresses gnu-line-marker warnings when invoking clang with input from a preprocessor job and the user has not otherwise explictly specified -Wgnu-line-marker somewhere on the command line. Note that this does apply to user provided preprocessed files.

fixes #63802

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: None (macurtis-amd)

Changes

When passing -save-temps to clang, the generated preprocessed output uses gnu line markers. This unexpectedly triggers gnu-line-marker warnings when used with -Weverything or -pedantic. Even worse, compilation fails if -Werror is used.

This change suppresses gnu-line-marker warnings when invoking clang with input from a preprocessor job and the user has not otherwise explictly specified -Wgnu-line-marker somewhere on the command line. Note that this does apply to user provided preprocessed files.

fixes #63802

---

Full diff: https://github.com/llvm/llvm-project/pull/134621.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+27)
• (modified) clang/test/Driver/save-temps.c (+16)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 70489adf01c94..36cfcecbabe98 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/CLWarnings.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "clang/Basic/HeaderInclude.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/MakeSupport.h"
@@ -67,6 +68,8 @@
 #include "llvm/TargetParser/RISCVTargetParser.h"
 #include <cctype>
 
+#include "/home/macurtis/mcc/src/MCC-PRINT.h"
+
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
@@ -5077,6 +5080,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Input =
       IsExtractAPI ? ExtractAPIPlaceholderInput : Inputs[0];
 
+  bool hasPreprocessorJobInput = false;
   InputInfoList ExtractAPIInputs;
   InputInfoList HostOffloadingInputs;
   const InputInfo *CudaDeviceInput = nullptr;
@@ -5101,6 +5105,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     } else {
       llvm_unreachable("unexpectedly given multiple inputs");
     }
+    hasPreprocessorJobInput |=
+        I.getAction()->getKind() == Action::PreprocessJobClass;
   }
 
   const llvm::Triple *AuxTriple =
@@ -6506,6 +6512,27 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.AddLastArg(CmdArgs, options::OPT_pedantic_errors);
   Args.AddLastArg(CmdArgs, options::OPT_w);
 
+  // Possibly suppress gnu-line-marker diagnostics. This suppresses spurious
+  // warnings or errors when using '-save-temps' with, for example, '-Werror
+  // -Weverything' or '-pedantic'.
+  if (hasPreprocessorJobInput &&
+      !D.getDiags().isIgnored(diag::ext_pp_gnu_line_directive,
+                              SourceLocation())) {
+    auto IDs = D.getDiags().getDiagnosticIDs();
+    StringRef gnuLineMarker =
+        IDs->getWarningOptionForDiag(diag::ext_pp_gnu_line_directive);
+
+    // If for some reason the user has explicitly specified -Wgnu-line-marker,
+    // they are on their own.
+    bool hasGnuLineMarker = false;
+    for (std::string &V : Args.getAllArgValues(options::OPT_W_Joined)) {
+      if ((hasGnuLineMarker = V == gnuLineMarker))
+        break;
+    }
+    if (!hasGnuLineMarker)
+      CmdArgs.push_back(Args.MakeArgString("-Wno-" + gnuLineMarker));
+  }
+
   Args.addOptInFlag(CmdArgs, options::OPT_ffixed_point,
                     options::OPT_fno_fixed_point);
 
diff --git a/clang/test/Driver/save-temps.c b/clang/test/Driver/save-temps.c
index b0cfa4fd814a8..48a590700076e 100644
--- a/clang/test/Driver/save-temps.c
+++ b/clang/test/Driver/save-temps.c
@@ -90,3 +90,19 @@
 // CHECK-SAVE-TEMPS-CMSE: -cc1as
 // CHECK-SAVE-TEMPS-CMSE: +8msecext
 // CHECK-SAVE-TEMPS-CMSE-NOT: '+cmse' is not a recognized feature for this target (ignoring feature)
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -Werror -pedantic -save-temps %s -### 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS-GLM-000
+// CHECK-SAVE-TEMPS-GLM-000: "-o" "save-temps.i"
+// CHECK-SAVE-TEMPS-GLM-000: "-pedantic" "-Wno-gnu-line-marker"{{.*}} "-x" "cpp-output" "save-temps.i"
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -Werror -Weverything -save-temps %s -### 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS-GLM-001
+// CHECK-SAVE-TEMPS-GLM-001: "-o" "save-temps.i"
+// CHECK-SAVE-TEMPS-GLM-001: "-Weverything" "-Wno-gnu-line-marker"{{.*}} "-x" "cpp-output" "save-temps.i"
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -Werror -Weverything -Wgnu-line-marker -save-temps %s -### 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS-GLM-002
+// RUN: %clang -target x86_64-unknown-linux-gnu -Werror -Weverything -save-temps -x cpp-output %s -### 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS-GLM-002
+// CHECK-SAVE-TEMPS-GLM-002-NOT: "-Wno-gnu-line-marker"

[MaskRay]

I am not sure clang driver should does this unusual suppression.

You'd also get a warning with: clang test.c -E -Wall -pedantic; clang test.i -S -Wall -pedantic

There might be a different strategy to suppress this diagnostic?

[macurtis-amd]

I am not sure clang driver should does this unusual suppression.

You'd also get a warning with: clang test.c -E -Wall -pedantic; clang test.i -S -Wall -pedantic

There might be a different strategy to suppress this diagnostic?

Did you have an alternative in mind?

Driver seemed like to best place to me because it is generating the commands for both the producer and consumer of the preprocessed file.

[MaskRay]

I don't, but I wonder what's the GCC behavior. It seems to suppress the diagnostic when processing a .i file.

[macurtis-amd]

I don't, but I wonder what's the GCC behavior. It seems to suppress the diagnostic when processing a .i file.

Diagnostic seems to be suppressed by -fpreprocessed, which is implicit if you pass a .i file to gcc and passed explicitly to cc1 when using -save-temps.

I guess the analogous clang option is -x?

[macurtis-amd]

@MaskRay Latest revision moves the suppression out of the Driver and into the Frontend. Behavior matches gcc.

[macurtis-amd]

@MaskRay Any other concerns with this change?

[macurtis-amd]

@MaskRay ping

[macurtis-amd]

@MaskRay I plan on merging this tomorrow morning unless you have any objections.

[MaskRay]

LGTM!