[clang] dc5cbba - [clang][modules] Add -Wsystem-headers-in-module=
Ben Langmuir via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 9 10:41:09 PDT 2023
Author: Ben Langmuir
Date: 2023-08-09T10:40:53-07:00
New Revision: dc5cbba3196db61d57b7d84118732a6c96d8ee64
URL: https://github.com/llvm/llvm-project/commit/dc5cbba3196db61d57b7d84118732a6c96d8ee64
DIFF: https://github.com/llvm/llvm-project/commit/dc5cbba3196db61d57b7d84118732a6c96d8ee64.diff
LOG: [clang][modules] Add -Wsystem-headers-in-module=
Add a way to enable -Wsystem-headers only for a specific module. This is
useful for validating a module that would otherwise not see system
header diagnostics without being flooded by diagnostics for unrelated
headers/modules. It's relatively common for a module to be marked
[system] but still wish to validate itself explicitly.
rdar://113401565
Differential Revision: https://reviews.llvm.org/D156948
Added:
clang/test/ClangScanDeps/Wsystem-headers-in-module.c
clang/test/Modules/Wsystem-headers-in-module.c
Modified:
clang/include/clang/Basic/DiagnosticOptions.h
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticOptions.h b/clang/include/clang/Basic/DiagnosticOptions.h
index 7e218b9c71e69e..0f3120859ecef6 100644
--- a/clang/include/clang/Basic/DiagnosticOptions.h
+++ b/clang/include/clang/Basic/DiagnosticOptions.h
@@ -123,6 +123,10 @@ class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{
/// default).
std::vector<std::string> VerifyPrefixes;
+ /// The list of -Wsystem-header-in-module=... options used to override
+ /// whether -Wsystem-headers is enabled on a per-module basis.
+ std::vector<std::string> SystemHeaderWarningsModules;
+
public:
// Define accessors/mutators for diagnostic options of enumeration type.
#define DIAGOPT(Name, Bits, Default)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0ad2fa0112328d..1166dd28b8331c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -843,6 +843,10 @@ def Wall : Flag<["-"], "Wall">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
def WCL4 : Flag<["-"], "WCL4">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
def Wsystem_headers : Flag<["-"], "Wsystem-headers">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
def Wno_system_headers : Flag<["-"], "Wno-system-headers">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
+def Wsystem_headers_in_module_EQ : Joined<["-"], "Wsystem-headers-in-module=">,
+ Flags<[CC1Option, HelpHidden]>, MetaVarName<"<module>">,
+ HelpText<"Enable -Wsystem-headers when building <module>">,
+ MarshallingInfoStringVector<DiagnosticOpts<"SystemHeaderWarningsModules">>;
def Wdeprecated : Flag<["-"], "Wdeprecated">, Group<W_Group>, Flags<[CC1Option]>,
HelpText<"Enable warnings for deprecated constructs and define __DEPRECATED">;
def Wno_deprecated : Flag<["-"], "Wno-deprecated">, Group<W_Group>, Flags<[CC1Option]>;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index ec01264bbc9926..00f2d7140db5d5 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5973,6 +5973,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
A->render(Args, CmdArgs);
}
+ Args.AddAllArgs(CmdArgs, options::OPT_Wsystem_headers_in_module_EQ);
+
if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false))
CmdArgs.push_back("-pedantic");
Args.AddLastArg(CmdArgs, options::OPT_pedantic_errors);
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 92e0b74e38f0af..0bbfb2a81acae6 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -39,6 +39,7 @@
#include "clang/Serialization/ASTReader.h"
#include "clang/Serialization/GlobalModuleIndex.h"
#include "clang/Serialization/InMemoryModuleCache.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Config/llvm-config.h"
@@ -1216,7 +1217,9 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
// Don't free the remapped file buffers; they are owned by our caller.
PPOpts.RetainRemappedFileBuffers = true;
- Invocation->getDiagnosticOpts().VerifyDiagnostics = 0;
+ DiagnosticOptions &DiagOpts = Invocation->getDiagnosticOpts();
+
+ DiagOpts.VerifyDiagnostics = 0;
assert(ImportingInstance.getInvocation().getModuleHash() ==
Invocation->getModuleHash() && "Module hash mismatch!");
@@ -1233,6 +1236,9 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
ImportingInstance.getDiagnosticClient()),
/*ShouldOwnClient=*/true);
+ if (llvm::is_contained(DiagOpts.SystemHeaderWarningsModules, ModuleName))
+ Instance.getDiagnostics().setSuppressSystemWarnings(false);
+
if (FrontendOpts.ModulesShareFileManager) {
Instance.setFileManager(&ImportingInstance.getFileManager());
} else {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index f6c643d9113d8a..483c632c9570c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -507,14 +507,17 @@ static bool isExtHandlingFromDiagsError(DiagnosticsEngine &Diags) {
}
static bool checkDiagnosticMappings(DiagnosticsEngine &StoredDiags,
- DiagnosticsEngine &Diags,
- bool IsSystem, bool Complain) {
+ DiagnosticsEngine &Diags, bool IsSystem,
+ bool SystemHeaderWarningsInModule,
+ bool Complain) {
// Top-level options
if (IsSystem) {
if (Diags.getSuppressSystemWarnings())
return false;
- // If -Wsystem-headers was not enabled before, be conservative
- if (StoredDiags.getSuppressSystemWarnings()) {
+ // If -Wsystem-headers was not enabled before, and it was not explicit,
+ // be conservative
+ if (StoredDiags.getSuppressSystemWarnings() &&
+ !SystemHeaderWarningsInModule) {
if (Complain)
Diags.Report(diag::err_pch_diagopt_mismatch) << "-Wsystem-headers";
return true;
@@ -586,10 +589,17 @@ bool PCHValidator::ReadDiagnosticOptions(
if (!TopM)
return false;
+ Module *Importer = PP.getCurrentModule();
+
+ DiagnosticOptions &ExistingOpts = ExistingDiags.getDiagnosticOptions();
+ bool SystemHeaderWarningsInModule =
+ Importer && llvm::is_contained(ExistingOpts.SystemHeaderWarningsModules,
+ Importer->Name);
+
// FIXME: if the diagnostics are incompatible, save a DiagnosticOptions that
// contains the union of their flags.
return checkDiagnosticMappings(*Diags, ExistingDiags, TopM->IsSystem,
- Complain);
+ SystemHeaderWarningsInModule, Complain);
}
/// Collect the macro definitions provided by the given preprocessor
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 478374b1e1e1b7..cadb1865731b04 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -12,6 +12,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/BLAKE3.h"
#include "llvm/Support/StringSaver.h"
#include <optional>
@@ -172,6 +173,13 @@ ModuleDepCollector::makeInvocationForModuleBuildWithoutOutputs(
CI.getHeaderSearchOpts().ModulesIgnoreMacros.clear();
}
+ // Apply -Wsystem-headers-in-module for the current module.
+ if (llvm::is_contained(CI.getDiagnosticOpts().SystemHeaderWarningsModules,
+ Deps.ID.ModuleName))
+ CI.getDiagnosticOpts().Warnings.push_back("system-headers");
+ // Remove the now unused option(s).
+ CI.getDiagnosticOpts().SystemHeaderWarningsModules.clear();
+
Optimize(CI);
return CI;
diff --git a/clang/test/ClangScanDeps/Wsystem-headers-in-module.c b/clang/test/ClangScanDeps/Wsystem-headers-in-module.c
new file mode 100644
index 00000000000000..98bdc4a8e13a84
--- /dev/null
+++ b/clang/test/ClangScanDeps/Wsystem-headers-in-module.c
@@ -0,0 +1,56 @@
+// Check that Wsystem-headers-in-module shows diagnostics in the named system
+// module, but not in other system headers or modules when built with explicit
+// modules.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/deps.json
+
+// RUN: %deps-to-rsp %t/deps.json --module-name=dependent_sys_mod > %t/dependent_sys_mod.rsp
+// RUN: %deps-to-rsp %t/deps.json --module-name=sys_mod > %t/sys_mod.rsp
+// RUN: %deps-to-rsp %t/deps.json --module-name=other_sys_mod > %t/other_sys_mod.rsp
+// RUN: %deps-to-rsp %t/deps.json --tu-index 0 > %t/tu.rsp
+
+// RUN: %clang @%t/dependent_sys_mod.rsp -verify
+// RUN: %clang @%t/sys_mod.rsp -verify
+// RUN: %clang @%t/other_sys_mod.rsp -verify
+// RUN: %clang @%t/tu.rsp -verify
+
+// CHECK-NOT: warning:
+// CHECK: sys_mod.h:2:7: warning: extra ';'
+// CHECK-NOT: warning:
+
+//--- cdb.json.template
+[{
+ "directory": "DIR",
+ "command": "clang -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/mcp DIR/tu.c -isystem DIR/sys -Wextra-semi -Wsystem-headers-in-module=sys_mod",
+ "file": "DIR/tu.c"
+}]
+
+//--- sys/other_sys_header.h
+int x;;
+
+//--- sys_mod.h
+#include "dependent_sys_mod.h"
+int y;; // expected-warning {{extra ';' outside of a function}}
+
+//--- other_sys_mod.h
+int z;;
+// expected-no-diagnostics
+
+//--- dependent_sys_mod.h
+int w;;
+// expected-no-diagnostics
+
+//--- module.modulemap
+module sys_mod [system] { header "sys_mod.h" }
+module other_sys_mod [system] { header "other_sys_mod.h" }
+module dependent_sys_mod [system] { header "dependent_sys_mod.h" }
+
+//--- tu.c
+#include "sys_mod.h"
+#include "other_sys_mod.h"
+#include "other_sys_header.h"
+// expected-no-diagnostics
diff --git a/clang/test/Modules/Wsystem-headers-in-module.c b/clang/test/Modules/Wsystem-headers-in-module.c
new file mode 100644
index 00000000000000..b156d291401a87
--- /dev/null
+++ b/clang/test/Modules/Wsystem-headers-in-module.c
@@ -0,0 +1,32 @@
+// Check that Wsystem-headers-in-module shows diagnostics in the named system
+// module, but not in other system headers or modules.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp \
+// RUN: -isystem %t/sys %t/tu.c -fsyntax-only -Wextra-semi -Wsystem-headers-in-module=sys_mod \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK-NOT: warning:
+// CHECK: sys_mod.h:2:7: warning: extra ';'
+// CHECK-NOT: warning:
+
+//--- sys/other_sys_header.h
+int x;;
+//--- sys_mod.h
+#include "dependent_sys_mod.h"
+int y;;
+//--- other_sys_mod.h
+int z;;
+//--- dependent_sys_mod.h
+int w;;
+//--- module.modulemap
+module sys_mod [system] { header "sys_mod.h" }
+module other_sys_mod [system] { header "other_sys_mod.h" }
+module dependent_sys_mod [system] { header "dependent_sys_mod.h" }
+
+//--- tu.c
+#include "sys_mod.h"
+#include "other_sys_mod.h"
+#include "other_sys_header.h"
More information about the cfe-commits
mailing list