[clang] f33173a - [clang][modules][deps] System module maps might not be affecting

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 1 22:41:46 PDT 2022


Author: Jan Svoboda
Date: 2022-11-01T22:41:40-07:00
New Revision: f33173acd6875fa86448bb6d91bdac8f1296510a

URL: https://github.com/llvm/llvm-project/commit/f33173acd6875fa86448bb6d91bdac8f1296510a
DIFF: https://github.com/llvm/llvm-project/commit/f33173acd6875fa86448bb6d91bdac8f1296510a.diff

LOG: [clang][modules][deps] System module maps might not be affecting

The dependency scanner relies on the module map filtering logic in `ASTWriter`. The algorithm currently considers all system module maps affecting, which is not only sub-optimal, but can also cause failures when building a module explicitly (see attached test case).

This patch applies the same filtering logic to system module maps.

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D136007

Added: 
    clang/test/ClangScanDeps/modules-redefinition.m

Modified: 
    clang/lib/Serialization/ASTWriter.cpp
    clang/test/ClangScanDeps/modules-implicit-dot-private.m
    clang/test/ClangScanDeps/modules-incomplete-umbrella.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 409f66c6e4da7..3658d69908c3c 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4544,7 +4544,7 @@ void ASTWriter::collectNonAffectingInputFiles() {
       continue;
 
     if (!isModuleMap(File.getFileCharacteristic()) ||
-        isSystem(File.getFileCharacteristic()) || AffectingModuleMaps.empty() ||
+        AffectingModuleMaps.empty() ||
         AffectingModuleMaps.find(Cache->OrigEntry) != AffectingModuleMaps.end())
       continue;
 

diff  --git a/clang/test/ClangScanDeps/modules-implicit-dot-private.m b/clang/test/ClangScanDeps/modules-implicit-dot-private.m
index 4f9a20d8872f2..20b988d15de32 100644
--- a/clang/test/ClangScanDeps/modules-implicit-dot-private.m
+++ b/clang/test/ClangScanDeps/modules-implicit-dot-private.m
@@ -15,7 +15,7 @@
 [{
   "file": "DIR/tu.m",
   "directory": "DIR",
-  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
 }]
 //--- tu.m
 @import FW.Private;
@@ -33,8 +33,7 @@
 // CHECK-NEXT:       "context-hash": "{{.*}}",
 // CHECK:            "file-deps": [
 // CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
-// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
-// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "FW"
 // CHECK:          },
@@ -45,7 +44,6 @@
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "{{.*}}",
 // CHECK:            "file-deps": [
-// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
 // CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
 // CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h"
 // CHECK-NEXT:       ],

diff  --git a/clang/test/ClangScanDeps/modules-incomplete-umbrella.c b/clang/test/ClangScanDeps/modules-incomplete-umbrella.c
index 989bf5f1e4ca7..58ac25179a318 100644
--- a/clang/test/ClangScanDeps/modules-incomplete-umbrella.c
+++ b/clang/test/ClangScanDeps/modules-incomplete-umbrella.c
@@ -26,7 +26,7 @@ framework module FW_Private {
 [{
   "file": "DIR/from_tu.m",
   "directory": "DIR",
-  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_tu.m -o DIR/from_tu.o"
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_tu.m -o DIR/from_tu.o"
 }]
 //--- from_tu.m
 #include "FW/FW.h"
@@ -45,8 +45,7 @@ framework module FW_Private {
 // CHECK_TU-NEXT:       "context-hash": "{{.*}}",
 // CHECK_TU-NEXT:       "file-deps": [
 // CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
-// CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
-// CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
 // CHECK_TU-NEXT:       ],
 // CHECK_TU-NEXT:       "name": "FW"
 // CHECK_TU-NEXT:     },
@@ -57,7 +56,6 @@ framework module FW_Private {
 // CHECK_TU:            ],
 // CHECK_TU-NEXT:       "context-hash": "{{.*}}",
 // CHECK_TU-NEXT:       "file-deps": [
-// CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
 // CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
 // CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
 // CHECK_TU-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/One.h"
@@ -102,7 +100,7 @@ framework module FW_Private {
 [{
   "file": "DIR/from_module.m",
   "directory": "DIR",
-  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -iframework DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -F DIR/frameworks -c DIR/from_module.m -o DIR/from_module.o"
 }]
 //--- module.modulemap
 module Mod { header "Mod.h" }
@@ -125,8 +123,7 @@ module Mod { header "Mod.h" }
 // CHECK_MODULE-NEXT:       "context-hash": "{{.*}}",
 // CHECK_MODULE-NEXT:       "file-deps": [
 // CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h",
-// CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
-// CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap"
+// CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap"
 // CHECK_MODULE-NEXT:       ],
 // CHECK_MODULE-NEXT:       "name": "FW"
 // CHECK_MODULE-NEXT:     },
@@ -137,7 +134,6 @@ module Mod { header "Mod.h" }
 // CHECK_MODULE:            ],
 // CHECK_MODULE-NEXT:       "context-hash": "{{.*}}",
 // CHECK_MODULE-NEXT:       "file-deps": [
-// CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap",
 // CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
 // CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/FW_Private.h",
 // CHECK_MODULE-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/One.h"

diff  --git a/clang/test/ClangScanDeps/modules-redefinition.m b/clang/test/ClangScanDeps/modules-redefinition.m
new file mode 100644
index 0000000000000..2125da5d3d887
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-redefinition.m
@@ -0,0 +1,60 @@
+// This test checks that we don't report non-affecting system module maps.
+// More specifically, we check that explicitly-specified module map file is not
+// being shadowed in explicit build by a module map file found during implicit
+// module map search.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- tu.m
+ at import first;
+
+//--- zeroth/module.modulemap
+module X {}
+
+//--- first/module.modulemap
+module first { header "first.h" }
+//--- first/first.h
+ at import third;
+//--- second/module.modulemap
+module X {}
+//--- third/module.modulemap
+module third {}
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.m",
+  "directory": "DIR",
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fmodule-name=X -fmodule-map-file=DIR/zeroth/module.modulemap -isystem DIR/first -isystem DIR/second -isystem DIR/third -c DIR/tu.m -o DIR/tu.o"
+}]
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+// CHECK:      {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": [
+// CHECK-NEXT:         {
+// CHECK-NEXT:           "context-hash": "{{.*}}",
+// CHECK-NEXT:           "module-name": "third"
+// CHECK-NEXT:         }
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT:       "command-line": [
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "{{.*}}",
+// CHECK-NEXT:       "file-deps": [
+// CHECK-NEXT:         [[PREFIX]]/first/first.h",
+// CHECK-NEXT:         [[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT:         [[PREFIX]]/third/module.modulemap"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "name": "first"
+// CHECK-NEXT:     }
+// CHECK:        ]
+// CHECK:      }
+
+// RUN: %deps-to-rsp %t/result.json --module-name=third > %t/third.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --module-name=first > %t/first.cc1.rsp
+// RUN: %clang @%t/third.cc1.rsp
+// RUN: %clang @%t/first.cc1.rsp


        


More information about the cfe-commits mailing list