From d759352db55420b54f594b6e358b3ff86fb7fb39 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 1 Nov 2022 21:57:42 -0700 Subject: [PATCH] [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 (cherry picked from commit f33173acd6875fa86448bb6d91bdac8f1296510a) --- clang/lib/Serialization/ASTWriter.cpp | 2 +- .../modules-implicit-dot-private.m | 6 +- .../modules-incomplete-umbrella.c | 12 ++-- .../test/ClangScanDeps/modules-redefinition.m | 60 +++++++++++++++++++ 4 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 clang/test/ClangScanDeps/modules-redefinition.m diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 66165fa4acf9ef0..5243f6da2fc339c 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4548,7 +4548,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 4f9a20d8872f22f..20b988d15de32f8 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 989bf5f1e4ca725..58ac25179a31872 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 000000000000000..2125da5d3d887ef --- /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 +@import first; + +//--- zeroth/module.modulemap +module X {} + +//--- first/module.modulemap +module first { header "first.h" } +//--- first/first.h +@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