Modules: Add -Rmodule-import

[dexonsmith]

Summary

Add a remark for importing modules. Depending on whether this is a
direct import (into the TU being built by this compiler instance) or
transitive import (into an already-imported module), the diagnostic has
two forms:

importing module 'Foo' from 'path/to/Foo.pcm'
importing module 'Foo' into 'Bar' from 'path/to/Foo.pcm'

Also drop a redundant FileCheck invocation in Rmodule-build.m that was
using -Reverything, since the notes from -Rmodule-import were confusing
it.

[dexonsmith]

clang/test/Modules/Rmodule-build.m

22–25

@bruno this looked redundant to me with the check above; can you confirm? (Now that we have -Rmodule-import, the FileCheck was failing...)

[bruno]

Nice, note that a subset of this information can be achieved with -Wauto-import, but this is way more general and solid, since it will handles @imports and pragma imports too. LGTM with a minor nitpick, see comments.

clang/test/Modules/Inputs/Rmodule-import/B.h

1

Can you make one of the tests to use @import (or the pragma version) instead of #include?

clang/test/Modules/Rmodule-build.m

22–25

Yes, it should be fine.

[dexonsmith]

Committed in r355477.

clang/test/Modules/Inputs/Rmodule-import/B.h

1

I changed the import of D.

clang/include/clang/Basic/DiagnosticGroups.td

 def LongLong : DiagGroup<"long-long", [CXX11LongLong]>;
 def ImplicitlyUnsignedLiteral : DiagGroup<"implicitly-unsigned-literal">;
 def MethodSignatures : DiagGroup<"method-signatures">;
 def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">;
 def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">;
 def MismatchedTags : DiagGroup<"mismatched-tags">;
 def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;
 def ModuleBuild : DiagGroup<"module-build">;
+def ModuleImport : DiagGroup<"module-import">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def ModuleFileExtension : DiagGroup<"module-file-extension">;
 def NewlineEOF : DiagGroup<"newline-eof">;
 def Nullability : DiagGroup<"nullability">;
 def NullabilityDeclSpec : DiagGroup<"nullability-declspec">;
 def NullabilityInferredOnNestedType : DiagGroup<"nullability-inferred-on-nested-type">;
 def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">;
 def NullabilityCompletenessOnArrays : DiagGroup<"nullability-completeness-on-arrays">;

clang/include/clang/Basic/DiagnosticSerializationKinds.td

def err_module_file_out_of_date : Error<
   "needs to be rebuilt%select{|: %3}2">, DefaultFatal;
 def err_module_file_invalid : Error<
   "file '%1' is not a valid precompiled %select{PCH|module|AST}0 file">, DefaultFatal;
 def note_module_file_imported_by : Note<
   "imported by %select{|module '%2' in }1'%0'">;
 def err_module_file_not_module : Error<
   "AST file '%0' was not built as a module">, DefaultFatal;
 
+def remark_module_import : Remark<
+  "importing module '%0'%select{| into '%3'}2 from '%1'">,
+  InGroup<ModuleImport>;
+
 def err_imported_module_not_found : Error<
     "module '%0' in AST file '%1' (imported by AST file '%2') "
     "is not defined in any loaded module map file; "
     "maybe you need to load '%3'?">, DefaultFatal;
 def note_imported_by_pch_module_not_found : Note<
     "consider adding '%0' to the header search path">;
 def err_imported_module_modmap_changed : Error<
     "module '%0' imported by AST file '%1' found in a different module map file"

clang/lib/Serialization/ASTReader.cpp

case ORIGINAL_FILE_ID:
       break;
 
     case ORIGINAL_PCH_DIR:
       F.OriginalDir = Blob;
       break;
 
     case MODULE_NAME:
       F.ModuleName = Blob;
+      Diag(diag::remark_module_import)
+          << F.ModuleName << F.FileName << (ImportedBy ? true : false)
+          << (ImportedBy ? StringRef(ImportedBy->ModuleName) : StringRef());
       if (Listener)
         Listener->ReadModuleName(F.ModuleName);
 
       // Validate the AST as soon as we have a name so we can exit early on
       // failure.
       if (ASTReadResult Result = readUnhashedControlBlockOnce())
         return Result;
 
ASTReader::ReadASTCore(StringRef FileName,
   ModuleManager::AddModuleResult AddResult
     = ModuleMgr.addModule(FileName, Type, ImportLoc, ImportedBy,
                           getGeneration(), ExpectedSize, ExpectedModTime,
                           ExpectedSignature, readASTFileSignature,
                           M, ErrorStr);
 
   switch (AddResult) {
   case ModuleManager::AlreadyLoaded:
+    Diag(diag::remark_module_import)
+        << M->ModuleName << M->FileName << (ImportedBy ? true : false)
+        << (ImportedBy ? StringRef(ImportedBy->ModuleName) : StringRef());
     return Success;
 
   case ModuleManager::NewlyLoaded:
     // Load module file below.
     break;
 
   case ModuleManager::Missing:
     // The module file was missing; if the client can handle that, return

clang/test/Modules/Inputs/Rmodule-import/A.h

+// A
+#include "B.h"

clang/test/Modules/Inputs/Rmodule-import/B.h

+// B
+#include "C.h"

clang/test/Modules/Inputs/Rmodule-import/C.h

+// C

clang/test/Modules/Inputs/Rmodule-import/D.h

+// D

clang/test/Modules/Inputs/Rmodule-import/module.modulemap

+module A { header "A.h" }
+module B { header "B.h" }
+module C { header "C.h" }
+module D { header "D.h" }

clang/test/Modules/Rmodule-build.m

 @import B; // expected-remark{{building module 'B' as}} expected-remark {{finished building module 'B'}}
 @import A; // no diagnostic
 @import B; // no diagnostic
 
 // RUN: echo ' ' >> %t/C.h
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fsyntax-only %s -I %t \
 // RUN:            -Rmodule-build 2>&1 | FileCheck %s
 
-// RUN: echo ' ' >> %t/C.h
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fsyntax-only %s -I %t \
-// RUN:            -Reverything 2>&1 | FileCheck %s
-
 // RUN: echo ' ' >> %t/B.h
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fsyntax-only %s -I %t \
 // RUN:            2>&1 | FileCheck -allow-empty -check-prefix=NO-REMARKS %s
 
 // RUN: echo ' ' >> %t/B.h
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fsyntax-only %s -I %t \
 // RUN:            -Rmodule-build -Rno-everything 2>&1 | \
 // RUN:    FileCheck -allow-empty -check-prefix=NO-REMARKS %s
 
 // CHECK-NOT: building module 'A'
 // CHECK: building module 'B'
 // CHECK: building module 'C'
 // CHECK: finished building module 'C'
 // CHECK: finished building module 'B'
 // NO-REMARKS-NOT: building module 'A'
 // NO-REMARKS-NOT: building module 'B'

clang/test/Modules/Rmodule-import.m

+// RUN: rm -rf %t1 %t2
+
+// Run with -verify, which onliy gets remarks from the main TU.
+//
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t1 \
+// RUN:     -fdisable-module-hash -fsyntax-only -I%S/Inputs/Rmodule-import \
+// RUN:     -Rmodule-build -Rmodule-import -verify %s
+
+// Run again, using FileCheck to check remarks from the module builds.
+//
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t2 \
+// RUN:     -fdisable-module-hash -fsyntax-only -I%S/Inputs/Rmodule-import \
+// RUN:     -Rmodule-build -Rmodule-import %s 2>&1 |\
+// RUN: FileCheck %s -implicit-check-not "remark:"
+
+#include "A.h" // \
+   expected-remark-re{{building module 'A' as '{{.*}}/A.pcm'}} \
+   expected-remark{{finished building module 'A'}} \
+   expected-remark-re{{importing module 'A' from '{{.*}}/A.pcm'}} \
+   expected-remark-re{{importing module 'B' into 'A' from '{{.*}}/B.pcm'}} \
+   expected-remark-re{{importing module 'C' into 'B' from '{{.*}}/C.pcm'}}
+// CHECK: remark: building module 'A'
+// CHECK: remark: building module 'B'
+// CHECK: remark: building module 'C'
+// CHECK: remark: finished building module 'C'
+// CHECK: remark: importing module 'C' from '{{.*}}/C.pcm'
+// CHECK: remark: finished building module 'B'
+// CHECK: remark: importing module 'B' from '{{.*}}/B.pcm'
+// CHECK: remark: importing module 'C' into 'B' from '{{.*}}/C.pcm'
+// CHECK: remark: finished building module 'A'
+// CHECK: remark: importing module 'A' from '{{.*}}/A.pcm'
+// CHECK: remark: importing module 'B' into 'A' from '{{.*}}/B.pcm'
+// CHECK: remark: importing module 'C' into 'B' from '{{.*}}/C.pcm'
+#include "B.h" // \
+   expected-remark-re{{importing module 'B' from '{{.*}}/B.pcm'}}
+// CHECK: remark: importing module 'B' from '{{.*}}/B.pcm'
+#include "C.h" // \
+   expected-remark-re{{importing module 'C' from '{{.*}}/C.pcm'}}
+// CHECK: remark: importing module 'C' from '{{.*}}/C.pcm'
+#include "D.h" // \
+   expected-remark-re{{building module 'D' as '{{.*}}/D.pcm'}} \
+   expected-remark{{finished building module 'D'}} \
+   expected-remark-re{{importing module 'D' from '{{.*}}/D.pcm'}}
+// CHECK: remark: building module 'D'
+// CHECK: remark: finished building module 'D'
+// CHECK: remark: importing module 'D' from '{{.*}}/D.pcm'