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

[clang][DependencyScanner] Remove all warning flags when suppressing warnings #71612

Merged
merged 2 commits into from
Nov 14, 2023

Conversation

Bigcheese
Copy link
Contributor

Since system modules don't emit most warnings, remove the warning flags to increase module reuse.

…warnings

Since system modules don't emit most warnings, remove the warning
flags to increase module reuse.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Nov 8, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 8, 2023

@llvm/pr-subscribers-clang

Author: Michael Spencer (Bigcheese)

Changes

Since system modules don't emit most warnings, remove the warning flags to increase module reuse.


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

4 Files Affected:

  • (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h (+5-2)
  • (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+26)
  • (added) clang/test/ClangScanDeps/optimize-system-warnings.m (+84)
  • (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+1)
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index a4a03b88b205175..dcdf1c171f6d731 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -51,8 +51,11 @@ enum class ScanningOptimizations {
   /// Remove unused header search paths including header maps.
   HeaderSearch = 1,
 
-  LLVM_MARK_AS_BITMASK_ENUM(HeaderSearch),
-  All = HeaderSearch,
+  /// Remove warnings from system modules.
+  SystemWarnings = 2,
+
+  LLVM_MARK_AS_BITMASK_ENUM(SystemWarnings),
+  All = HeaderSearch | SystemWarnings,
   Default = All
 };
 
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 5d95bb305bc38d8..9099c18391e4d29 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -52,6 +52,28 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions &Opts,
     Opts.UserEntries.push_back(Entries[Idx]);
 }
 
+static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
+                                   bool IsSystemModule) {
+  // If this is not a system module or -Wsystem-headers was passed, don't
+  // optimize.
+  if (!IsSystemModule)
+    return;
+  bool Wsystem_headers = false;
+  for (StringRef Opt : Opts.Warnings) {
+    bool isPositive = !Opt.consume_front("no-");
+    if (Opt == "system-headers")
+      Wsystem_headers = isPositive;
+  }
+  if (Wsystem_headers)
+    return;
+
+  // Remove all warning flags. System modules suppress most, but not all,
+  // warnings.
+  Opts.Warnings.clear();
+  Opts.UndefPrefixes.clear();
+  Opts.Remarks.clear();
+}
+
 static std::vector<std::string> splitString(std::string S, char Separator) {
   SmallVector<StringRef> Segments;
   StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, /*KeepEmpty=*/false);
@@ -532,6 +554,10 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
             if (any(MDC.OptimizeArgs & ScanningOptimizations::HeaderSearch))
               optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
                                        *MDC.ScanInstance.getASTReader(), *MF);
+            if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
+              optimizeDiagnosticOpts(
+                  BuildInvocation.getMutDiagnosticOpts(),
+                  BuildInvocation.getFrontendOpts().IsSystemModule);
           });
 
   MDC.associateWithContextHash(CI, MD);
diff --git a/clang/test/ClangScanDeps/optimize-system-warnings.m b/clang/test/ClangScanDeps/optimize-system-warnings.m
new file mode 100644
index 000000000000000..d61724c6f18fff8
--- /dev/null
+++ b/clang/test/ClangScanDeps/optimize-system-warnings.m
@@ -0,0 +1,84 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > %t/build/compile-commands.json
+// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
+// RUN:   -j 1 -format experimental-full -optimize-args=system-warnings > %t/deps.db
+// RUN: cat %t/deps.db | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK:      {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": [],
+// CHECK-NEXT:       "clang-modulemap-file":
+// CHECK-NEXT:       "command-line": [
+// CHECK-NOT:          "-W
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "{{.*}}",
+// CHECK-NEXT:       "file-deps": [
+// CHECK:            ],
+// CHECK-NEXT:       "name": "A"
+// CHECK-NEXT:     },
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": [],
+// CHECK-NEXT:       "clang-modulemap-file":
+// CHECK-NEXT:       "command-line": [
+// CHECK-NOT:          "-W
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "{{.*}}",
+// CHECK-NEXT:       "file-deps": [
+// CHECK:            ],
+// CHECK-NEXT:       "name": "B"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK:        ]
+// CHECK:      }
+
+//--- build/compile-commands.json.in
+
+[
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/A.m -isystem modules/A -I modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
+  "file": "DIR/A.m"
+},
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/B.m -isystem modules/A -I modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps -Wmaybe-unused",
+  "file": "DIR/B.m"
+}
+]
+
+//--- modules/A/module.modulemap
+
+module A {
+  umbrella header "A.h"
+}
+
+//--- modules/A/A.h
+
+typedef int A_t;
+
+//--- modules/B/module.modulemap
+
+module B [system] {
+  umbrella header "B.h"
+}
+
+//--- modules/B/B.h
+
+typedef int B_t;
+
+//--- A.m
+
+#include <A.h>
+#include <B.h>
+
+A_t a = 0;
+
+//--- B.m
+
+#include <A.h>
+#include <B.h>
+
+A_t b = 0;
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 50631fdf0bc3b75..f11c933d9576565 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -156,6 +156,7 @@ static void ParseArgs(int argc, char **argv) {
         llvm::StringSwitch<std::optional<ScanningOptimizations>>(Arg)
             .Case("none", ScanningOptimizations::None)
             .Case("header-search", ScanningOptimizations::HeaderSearch)
+            .Case("system-warnings", ScanningOptimizations::SystemWarnings)
             .Case("all", ScanningOptimizations::All)
             .Default(std::nullopt);
     if (!Optimization) {

@Bigcheese
Copy link
Contributor Author

Added test comments and added a test for -Wsystem-headers being present.

Copy link
Contributor

@jansvoboda11 jansvoboda11 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Bigcheese Bigcheese merged commit 731152e into llvm:main Nov 14, 2023
3 checks passed
jansvoboda11 added a commit to apple/llvm-project that referenced this pull request Nov 16, 2023
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…warnings (llvm#71612)

Since system modules don't emit most warnings, remove the warning flags
to increase module reuse.
Bigcheese pushed a commit to Bigcheese/llvm-project that referenced this pull request Nov 30, 2023
This started failing after llvm#71612.

(cherry picked from commit 07d662d)
Bigcheese added a commit to Bigcheese/llvm-project that referenced this pull request Nov 30, 2023
…warnings (llvm#71612)

Since system modules don't emit most warnings, remove the warning flags
to increase module reuse.

(cherry picked from commit 731152e)
Bigcheese pushed a commit to apple/llvm-project that referenced this pull request Dec 1, 2023
This started failing after llvm#71612.

(cherry picked from commit 07d662d)
Bigcheese added a commit to apple/llvm-project that referenced this pull request Dec 1, 2023
…warnings (llvm#71612)

Since system modules don't emit most warnings, remove the warning flags
to increase module reuse.

(cherry picked from commit 731152e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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