[C++20] [Modules] Emit a warning if the we load the modules by implic…

…it generated path A step to address #62707. It is not user friendly enough to drop the implicitly generated path directly. Let's emit the warning first and drop it in the next version.
llvm · May 17, 2023 · e22fa1d · e22fa1d
1 parent ed90cf1
commit e22fa1d
Show file tree

Hide file tree

Showing 17 changed files with 94 additions and 32 deletions.
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -133,6 +133,11 @@ def warn_eagerly_load_for_standard_cplusplus_modules : Warning<
   "the form '-fmodule-file=<BMI-path>' is deprecated for standard C++ named modules;"
   "consider to use '-fmodule-file=<module-name>=<BMI-path>' instead">,
   InGroup<DiagGroup<"eager-load-cxx-named-modules">>;
+
+def warn_reading_std_cxx_module_by_implicit_paths : Warning<
+  "it is deprecated to read module '%0' implcitly; it is going to be removed in clang18; "
+  "consider to specify the dependencies explicitly">,
+  InGroup<DiagGroup<"read-modules-implicitly">>;
 } // let CategoryName
 
 let CategoryName = "AST Serialization Issue" in {

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
@@ -2874,6 +2874,8 @@ ASTReader::ReadControlBlock(ModuleFile &F,
       while (Idx < N) {
         // Read information about the AST file.
         ModuleKind ImportedKind = (ModuleKind)Record[Idx++];
+        // Whether we're importing a standard c++ module.
+        bool IsImportingStdCXXModule = Record[Idx++];
         // The import location will be the local one for now; we will adjust
         // all import locations of module imports after the global source
         // location info are setup, in ReadAST.
@@ -2891,18 +2893,25 @@ ASTReader::ReadControlBlock(ModuleFile &F,
 
         // For prebuilt and explicit modules first consult the file map for
         // an override. Note that here we don't search prebuilt module
-        // directories, only the explicit name to file mappings. Also, we will
-        // still verify the size/signature making sure it is essentially the
-        // same file but perhaps in a different location.
+        // directories if we're not importing standard c++ module, only the
+        // explicit name to file mappings. Also, we will still verify the
+        // size/signature making sure it is essentially the same file but
+        // perhaps in a different location.
         if (ImportedKind == MK_PrebuiltModule || ImportedKind == MK_ExplicitModule)
           ImportedFile = PP.getHeaderSearchInfo().getPrebuiltModuleFileName(
-            ImportedName, /*FileMapOnly*/ true);
+              ImportedName, /*FileMapOnly*/ !IsImportingStdCXXModule);
+
+        if (ImportedFile.empty()) {
+          // It is deprecated for C++20 Named modules to use the implicitly
+          // paths.
+          if (IsImportingStdCXXModule)
+            Diag(clang::diag::warn_reading_std_cxx_module_by_implicit_paths)
+                << ImportedName;
 
-        if (ImportedFile.empty())
           // Use BaseDirectoryAsWritten to ensure we use the same path in the
           // ModuleCache as when writing.
           ImportedFile = ReadPath(BaseDirectoryAsWritten, Record, Idx);
-        else
+        } else
           SkipPath(Record, Idx);
 
         // If our client can't cope with us being out of date, we can't cope with
@@ -5445,9 +5454,9 @@ bool ASTReader::readASTFileControlBlock(
       unsigned Idx = 0, N = Record.size();
       while (Idx < N) {
         // Read information about the AST file.
-        Idx +=
-            1 + 1 + 1 + 1 +
-            ASTFileSignature::size; // Kind, ImportLoc, Size, ModTime, Signature
+
+        // Kind, StandardCXXModule, ImportLoc, Size, ModTime, Signature
+        Idx += 1 + 1 + 1 + 1 + 1 + ASTFileSignature::size;
         std::string ModuleName = ReadString(Record, Idx);
         std::string Filename = ReadString(Record, Idx);
         ResolveImportedPath(Filename, ModuleDir);

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
@@ -1352,6 +1352,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
         continue;
 
       Record.push_back((unsigned)M.Kind); // FIXME: Stable encoding
+      Record.push_back(M.StandardCXXModule);
       AddSourceLocation(M.ImportLoc, Record);
 
       // If we have calculated signature, there is no need to store

diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -634,6 +634,9 @@ llvm::Error GlobalModuleIndexBuilder::loadModuleFile(const FileEntry *File) {
         // Skip the imported kind
         ++Idx;
 
+        // Skip if it is standard C++ module
+        ++Idx;
+
         // Skip the import location
         ++Idx;
 

diff --git a/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp b/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
@@ -8,7 +8,7 @@
 // RUN:   -fmodule-file=M=M.pcm
 // RUN: %clang_cc1 -std=c++20 Q.cpp -emit-module-interface -o Q.pcm
 // RUN: %clang_cc1 -std=c++20 Q-impl.cpp -fsyntax-only -fmodule-file=Q=Q.pcm \
-// RUN:   -fmodule-file=N=N.pcm -verify
+// RUN:   -fmodule-file=N=N.pcm -fmodule-file=M=M.pcm -verify
 
 //--- M.cpp
 export module M;

diff --git a/clang/test/CXX/module/basic/basic.search/module-import.cppm b/clang/test/CXX/module/basic/basic.search/module-import.cppm
@@ -10,7 +10,7 @@
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=x=%t/x.pcm -verify %t/use.cpp \
 // RUN:            -DMODULE_NAME=x
 // RUN: %clang_cc1 -std=c++20 -I%t -fmodule-file=y=%t/y.pcm -verify %t/use.cpp \
-// RUN:            -DMODULE_NAME=y
+// RUN:            -DMODULE_NAME=y -fmodule-file=x=%t/x.pcm
 //
 // RUN: mv %t/x.pcm %t/a.pcm
 //

diff --git a/clang/test/CXX/module/module.context/p7.cpp b/clang/test/CXX/module/module.context/p7.cpp
@@ -12,13 +12,14 @@
 // RUN: -o stuff.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-6-ex1-M1.cpp \
-// RUN: -fmodule-file=stuff=stuff.pcm -o M1.pcm  -fmodule-file=defn.pcm
+// RUN:   -fmodule-file=stuff=stuff.pcm -o M1.pcm  -fmodule-file=defn.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-6-ex1-M2.cpp \
-// RUN: -fmodule-file=stuff=stuff.pcm -o M2.pcm  -fmodule-file=decl.pcm
+// RUN:   -fmodule-file=stuff=stuff.pcm -o M2.pcm  -fmodule-file=decl.pcm
 
 // RUN: %clang_cc1 -std=c++20 std-10-6-ex1-use.cpp \
-// RUN: -fmodule-file=M1=M1.pcm -fmodule-file=M2=M2.pcm  -fsyntax-only -verify
+// RUN:   -fmodule-file=M1=M1.pcm -fmodule-file=M2=M2.pcm -fmodule-file=stuff=stuff.pcm \
+// RUN:   -fsyntax-only -verify
 
 //--- std-10-6-ex1-decl.h
 struct X;

diff --git a/clang/test/CXX/module/module.interface/p2.cpp b/clang/test/CXX/module/module.interface/p2.cpp
@@ -4,8 +4,10 @@
 // RUN: %clang_cc1 -std=c++20 %s -DX_INTERFACE -emit-module-interface -o %t/x.pcm
 // RUN: %clang_cc1 -std=c++20 %s -DY_INTERFACE -emit-module-interface -o %t/y.pcm
 // RUN: %clang_cc1 -std=c++20 %s -DINTERFACE -fmodule-file=X=%t/x.pcm -fmodule-file=Y=%t/y.pcm -emit-module-interface -o %t/m.pcm
-// RUN: %clang_cc1 -std=c++20 %s -DIMPLEMENTATION -I%S/Inputs -fmodule-file=%t/h.pcm -fmodule-file=p2=%t/m.pcm -verify
-// RUN: %clang_cc1 -std=c++20 %s -DUSER -I%S/Inputs -fmodule-file=%t/h.pcm -fmodule-file=p2=%t/m.pcm -verify
+// RUN: %clang_cc1 -std=c++20 %s -DIMPLEMENTATION -I%S/Inputs -fmodule-file=%t/h.pcm \
+// RUN:   -fmodule-file=X=%t/x.pcm -fmodule-file=Y=%t/y.pcm -fmodule-file=p2=%t/m.pcm -verify
+// RUN: %clang_cc1 -std=c++20 %s -DUSER -I%S/Inputs -fmodule-file=%t/h.pcm -fmodule-file=p2=%t/m.pcm \
+// RUN:   -fmodule-file=X=%t/x.pcm -fmodule-file=Y=%t/y.pcm -verify
 
 #if defined(X_INTERFACE)
 export module X;

diff --git a/clang/test/Modules/cxx20-10-1-ex2.cpp b/clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -7,22 +7,23 @@
 // RUN:  -o %t/B_Y.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-1-ex2-tu2.cpp \
-// RUN:  -fmodule-file=B:Y=%t/B_Y.pcm -o %t/B.pcm
+// RUN:     -fmodule-file=B:Y=%t/B_Y.pcm -o %t/B.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-1-ex2-tu3.cpp \
-// RUN:   -o %t/B_X1.pcm -verify
+// RUN:     -o %t/B_X1.pcm -verify
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-1-ex2-tu4.cpp \
-// RUN:-fmodule-file=B=%t/B.pcm  -o %t/B_X2.pcm
+// RUN:     -fmodule-file=B=%t/B.pcm -fmodule-file=B:Y=%t/B_Y.pcm  -o %t/B_X2.pcm
 
 // RUN: %clang_cc1 -std=c++20 -emit-obj %t/std10-1-ex2-tu5.cpp \
-// RUN:  -fmodule-file=B=%t/B.pcm  -o %t/b_tu5.o
+// RUN:     -fmodule-file=B=%t/B.pcm -fmodule-file=B:Y=%t/B_Y.pcm  -o %t/b_tu5.o
 
 // RUN: %clang_cc1 -std=c++20 -S %t/std10-1-ex2-tu6.cpp \
-// RUN:  -fmodule-file=B=%t/B.pcm  -o %t/b_tu6.s -verify
+// RUN:     -fmodule-file=B=%t/B.pcm -fmodule-file=B:Y=%t/B_Y.pcm  -o %t/b_tu6.s -verify
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/std10-1-ex2-tu7.cpp \
-// RUN: -fmodule-file=B:X2=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
+// RUN:     -fmodule-file=B:X2=%t/B_X2.pcm -fmodule-file=B=%t/B.pcm \
+// RUN:     -fmodule-file=B:Y=%t/B_Y.pcm   -o %t/B_X3.pcm -verify
 
 //--- std10-1-ex2-tu1.cpp
 module B:Y;

diff --git a/clang/test/Modules/cxx20-import-diagnostics-a.cpp b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -12,7 +12,8 @@
 // RUN:  -fmodule-file=B=%t/B.pcm -fmodule-file=C=%t/C.pcm -o %t/AOK1.pcm
 
 // RUN: %clang_cc1 -std=c++20 -S %t/import-diags-tu4.cpp \
-// RUN:  -fmodule-file=AOK1=%t/AOK1.pcm -fmodule-file=C=%t/C.pcm -o %t/tu_3.s -verify
+// RUN:  -fmodule-file=AOK1=%t/AOK1.pcm -fmodule-file=B=%t/B.pcm \
+// RUN:  -fmodule-file=C=%t/C.pcm -o %t/tu_3.s -verify
 
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/import-diags-tu5.cpp \
 // RUN:  -fmodule-file=B=%t/B.pcm -fmodule-file=C=%t/C.pcm -o %t/BC.pcm -verify

diff --git a/clang/test/Modules/eagerly-load-cxx-named-modules.cppm b/clang/test/Modules/eagerly-load-cxx-named-modules.cppm
@@ -7,7 +7,8 @@
 // RUN:    2>&1 | FileCheck %t/user.cpp
 // RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
 // RUN:    -fprebuilt-module-path=%t
-// RUN: %clang_cc1 -std=c++20 %t/b.pcm -S -emit-llvm 2>&1 -o - | FileCheck %t/b.cppm
+// RUN: %clang_cc1 -std=c++20 %t/b.pcm -Wno-read-modules-implicitly -S \
+// RUN:    -emit-llvm 2>&1 -o - | FileCheck %t/b.cppm
 
 //--- a.cppm
 export module a;

diff --git a/clang/test/Modules/implicit-module-with-missing-path.cpp b/clang/test/Modules/implicit-module-with-missing-path.cpp
@@ -6,7 +6,7 @@
 // RUN: echo -e "export module B;\nimport C;" >> %t/B.cppm
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/subdir/C.cppm -o %t/subdir/C.pcm
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t/subdir %t/B.cppm -o %t/B.pcm
-// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -fsyntax-only -verify -Wno-read-modules-implicitly
 
 import B;
 import C; // expected-error {{module 'C' is needed but has not been provided, and implicit use of module files is disabled}}
diff --git a/clang/test/Modules/named-modules-adl-2.cppm b/clang/test/Modules/named-modules-adl-2.cppm
@@ -4,7 +4,7 @@
 //
 // RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
 // RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.pcm -emit-module-interface -o %t/b.pcm
-// RUN: %clang_cc1 -std=c++20 %t/c.cppm -fmodule-file=b=%t/b.pcm -fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm -fsyntax-only -verify
 
 //--- a.cppm
 export module a;

diff --git a/clang/test/Modules/no-implicit-std-cxx-module.cppm b/clang/test/Modules/no-implicit-std-cxx-module.cppm
@@ -0,0 +1,36 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/b.cppm -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -fmodule-file=b=%t/b.pcm \
+// RUN:     -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/user.cpp -fmodule-file=a=%t/a.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/user.cpp -fmodule-file=a=%t/a.pcm -verify -fsyntax-only \
+// RUN:     -Wno-read-modules-implicitly -DNO_DIAG 
+// RUN: %clang_cc1 -std=c++20 %t/user.cpp -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
+// RUN:     -DNO_DIAG -verify -fsyntax-only
+
+//--- b.cppm
+export module b;
+export int b() {
+    return 43;
+}
+
+//--- a.cppm
+export module a;
+import b;
+export int a() {
+    return b() + 43;
+}
+
+//--- user.cpp
+#ifdef NO_DIAG
+// expected-no-diagnostics
+#else
+ // expected-warning@+2 {{it is deprecated to read module 'b' implcitly;}}
+#endif
+import a;
+int use() {
+    return a();
+}
diff --git a/clang/test/Modules/pr56916.cppm b/clang/test/Modules/pr56916.cppm
@@ -6,8 +6,7 @@
 // RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-module-interface -o %t/M-B.pcm
 // RUN: %clang_cc1 -std=c++20 %t/M.cppm -emit-module-interface -o %t/M.pcm \
 // RUN:     -fprebuilt-module-path=%t
-// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fmodule-file=M=%t/M.pcm -fsyntax-only \
-// RUN:     -verify
+// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fsyntax-only -fprebuilt-module-path=%t -verify
 
 //--- foo.h
 template <typename T>

diff --git a/clang/test/Modules/pr60036.cppm b/clang/test/Modules/pr60036.cppm
@@ -17,9 +17,12 @@
 // RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -fmodule-file=a=%t/a.pcm -o %t/b.pcm
 // RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -fmodule-file=a=%t/a.pcm -o %t/c.pcm
 // RUN: %clang_cc1 -std=c++20 %t/d.cppm -emit-module-interface  -o %t/d.pcm
-// RUN: %clang_cc1 -std=c++20 %t/e.cppm -emit-module-interface -fmodule-file=d=%t/d.pcm -o %t/e.pcm
+// RUN: %clang_cc1 -std=c++20 %t/e.cppm -emit-module-interface -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
+// RUN:		-fmodule-file=c=%t/c.pcm -fmodule-file=d=%t/d.pcm -o %t/e.pcm
 // RUN: %clang_cc1 -std=c++20 %t/f.cppm -emit-module-interface -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm -fmodule-file=c=%t/c.pcm -fmodule-file=d=%t/d.pcm -o %t/f.pcm
-// RUN: %clang_cc1 -std=c++20 %t/g.cppm -fmodule-file=e=%t/e.pcm -fmodule-file=f=%t/f.pcm -verify -fsyntax-only 
+// RUN: %clang_cc1 -std=c++20 %t/g.cppm -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
+// RUN:		-fmodule-file=c=%t/c.pcm -fmodule-file=d=%t/d.pcm -fmodule-file=e=%t/e.pcm \
+// RUN:		-fmodule-file=f=%t/f.pcm -verify -fsyntax-only 
 
 //--- a.cppm
 export module a;

diff --git a/clang/test/Modules/pr60775.cppm b/clang/test/Modules/pr60775.cppm
@@ -8,9 +8,9 @@
 // RUN: %clang_cc1 -std=c++20 %t/b.cpp -fmodule-file=a=%t/a.pcm -verify -fsyntax-only
 // RUN: %clang_cc1 -std=c++20 %t/c.cppm -I%t -emit-module-interface -o %t/c.pcm
 // RUN: %clang_cc1 -std=c++20 %t/d.cppm -emit-module-interface -fmodule-file=c=%t/c.pcm -o %t/d.pcm
-// RUN: %clang_cc1 -std=c++20 %t/e.cpp -fmodule-file=d=%t/d.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/e.cpp -fmodule-file=d=%t/d.pcm -fmodule-file=c=%t/c.pcm -verify -fsyntax-only
 // RUN: %clang_cc1 -std=c++20 %t/f.cppm -emit-module-interface -fmodule-file=c=%t/c.pcm -o %t/f.pcm
-// RUN: %clang_cc1 -std=c++20 %t/g.cpp -fmodule-file=f=%t/f.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/g.cpp -fmodule-file=f=%t/f.pcm -fmodule-file=c=%t/c.pcm  -verify -fsyntax-only
 
 //--- initializer_list.h
 namespace std {