This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc5cbba3196d: [clang][modules] Add
-Wsystem-headers-in-module= (authored by benlangmuir).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156948/new/
https://reviews.llvm.org/D156948
Files:
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
clang/test/ClangScanDeps/Wsystem-headers-in-module.c
clang/test/Modules/Wsystem-headers-in-module.c
Index: clang/test/Modules/Wsystem-headers-in-module.c
===================================================================
--- /dev/null
+++ 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"
Index: clang/test/ClangScanDeps/Wsystem-headers-in-module.c
===================================================================
--- /dev/null
+++ 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
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ 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 @@
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;
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -507,14 +507,17 @@
}
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 @@
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
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ 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 @@
// 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 @@
ImportingInstance.getDiagnosticClient()),
/*ShouldOwnClient=*/true);
+ if (llvm::is_contained(DiagOpts.SystemHeaderWarningsModules, ModuleName))
+ Instance.getDiagnostics().setSuppressSystemWarnings(false);
+
if (FrontendOpts.ModulesShareFileManager) {
Instance.setFileManager(&ImportingInstance.getFileManager());
} else {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5973,6 +5973,8 @@
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);
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -843,6 +843,10 @@
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]>;
Index: clang/include/clang/Basic/DiagnosticOptions.h
===================================================================
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -123,6 +123,10 @@
/// 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)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits