[Modules] Add module structure output to -module-file-info.

[iains]

Summary

It is useful to be able to visualise the C++20 modules content of a PCM file
both for inspection and for testing. In particular, when adding more module
types to support C++20 Partitions and Header Units, we would like to be able
to confirm that the output PCM has the intended structure.

The existing scheme for dumping data is restricted to the content of the AST
file control block, which does not include structural data beyond imports.

The change here makes use of the AST unit that is set up by BeginSourceFile
to query for the information on the primary and sub-modules. We can then
inspect each of these in turn, accounting for Global, Private, Imported and
Exported modules/fragments and then showing the sub-stucture of the main
module(s).

The disadvantage of this mechanism is that it has no easy method to control
the granularity of the output. Perhaps more detailed inspection would be
better handled by a stand-alone module inspection tool.

[urnathan]

clang/lib/Frontend/FrontendActions.cpp

817–824

belongs in a later patch?

[iains]

address review comment, fix formatting.

[ChuanqiXu]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

According to P1857R3, it might not be good to add macro declarative before module declaration. Although clang didn't implement it and there are old test case uses this style, I think it might be better to split into files. @urnathan how do you think about this?

[iains]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

I see this in the Tony tables, but OTOH I cannot exactly see where the wording forbids it - in fact it permits preprocesssor directives before the import (including in the example in 5.7)... perhaps I'm misreading.

This way of writing the test cases does make them much easier to manage (and for a reader to see the intent) - of course, if we should split into files - then that is what we should do.

[ChuanqiXu]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

I think the grammar in [[ http://eel.is/c++draft/cpp | [cpp.pre] ]] forbid it.

First, the file could only be:

preprocessing-file:
group_opt
module-file

Then it is clear that we couldn't put macro declarative before module declaration. I agree the current style is easier and more convenient. In fact, I prefer it too. But I think we would better follow it since it is standard. Otherwise, it would be more work in later maintenance stage.

[urnathan]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

ChuanqiXu is correct about preprocessor directives not being allowed before the initial module decl, but I don;t think compilers implement that. There are a couple of issues.

a) some users have a need to have #pragma charset-or-similar before any tokens
b) forced headers

I do find the #if use here somewhat confusing. A bunch of cxx20-file-info-[1-n].cpp might be more straightforwards? (It does seem that directory is using cxx20 as a prefix rather than suffix btw.)

[urnathan]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

or maybe splitting this up make it harder to understand the tests as a whole. I don;t think it terribly importand. If we do ban pre-module directives, we can always split it then,

[iains]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

OK ( actually, I find it easier to grok the test case intent if the various required inputs are built in order, in the single file - but that's the nature of preferences :) )

I suppose that (a) and (b) mean that we cannot reasonably diagnose this?

Probably some of the files (e.g. ones making dummy modules to be consumed by the actual tests) could be in 'Inputs'. Will rework this accordingly.

[urnathan]

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

I suppose that (a) and (b) mean that we cannot reasonably diagnose this?

The #pragma item came up in stdization and IIRC there was some handwaving that such a pragma is not actually in the source text -- because it's telling you something about the encoding. One could probably make a similar argument about forced headers -- they're implementation detail (that the user might be able to control).

Anyway, IMHO this is orthogonal, and we might even be able to deploy something like:

#pragma GCC system_header
#look ma! random directive
module;

if we do start being pickier.

[ChuanqiXu]

LGTM.

clang/test/Modules/module-file-info-cxx20.cpp

26 ↗

(On Diff #408930)

OK, so I summarize the conclusion here is to allow use of macro declarative in test now and we could refactor in one shot once compiler implement the constraint. It is good for me as long as we get in consensus.

[iains]

rebased, amended testcases to use split-file, handle partition enumerations.

[iains]

this now uses the split-file approach to the test case so that we have the test case self-contained, but avoid the use of preprocessor directives before the module lines.

[iains]

reformat.

clang/lib/Frontend/FrontendActions.cpp

 //===--- FrontendActions.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Lex/DependencyDirectivesSourceMinimizer.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "clang/Serialization/ModuleFile.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
 #include <system_error>

 
 bool DumpModuleInfoAction::BeginInvocation(CompilerInstance &CI) {
   // The Object file reader also supports raw ast files and there is no point in
   // being strict about the module file format in -module-file-info mode.
   CI.getHeaderSearchOpts().ModuleFormat = "obj";
   return true;
 }
 
+static StringRef ModuleKindName(Module::ModuleKind MK) {
+  switch (MK) {
+  case Module::ModuleMapModule:
+    return "Module Map Module";
+  case Module::ModuleInterfaceUnit:
+    return "Interface Unit";
+  case Module::ModulePartitionInterface:
+    return "Partition Interface";
+  case Module::ModulePartitionImplementation:
+    return "Partition Implementation";
+  case Module::GlobalModuleFragment:
+    return "Global Module Fragment";
+  case Module::PrivateModuleFragment:
+    return "Private Module Fragment";
+  }
+}
+
 void DumpModuleInfoAction::ExecuteAction() {
+  assert(isCurrentFileAST() && "dumping non-AST?");
   // Set up the output file.
   std::unique_ptr<llvm::raw_fd_ostream> OutFile;
   StringRef OutputFileName = getCompilerInstance().getFrontendOpts().OutputFile;
   if (!OutputFileName.empty() && OutputFileName != "-") {
     std::error_code EC;
     OutFile.reset(new llvm::raw_fd_ostream(OutputFileName.str(), EC,
                                            llvm::sys::fs::OF_TextWithCRLF));
   }
   llvm::raw_ostream &Out = OutFile.get()? *OutFile.get() : llvm::outs();
 
   Out << "Information for module file '" << getCurrentFile() << "':\n";
   auto &FileMgr = getCompilerInstance().getFileManager();
   auto Buffer = FileMgr.getBufferForFile(getCurrentFile());
   StringRef Magic = (*Buffer)->getMemBufferRef().getBuffer();
   bool IsRaw = (Magic.size() >= 4 && Magic[0] == 'C' && Magic[1] == 'P' &&
                 Magic[2] == 'C' && Magic[3] == 'H');
   Out << "  Module format: " << (IsRaw ? "raw" : "obj") << "\n";
 
   Preprocessor &PP = getCompilerInstance().getPreprocessor();
   DumpModuleInfoListener Listener(Out);
-  HeaderSearchOptions &HSOpts =
-      PP.getHeaderSearchInfo().getHeaderSearchOpts();
+  HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
+
+  // The FrontendAction::BeginSourceFile () method loads the AST so that much
+  // of the information is already available and modules should have been
+  // loaded.
+
+  const LangOptions &LO = getCurrentASTUnit().getLangOpts();
+  if (LO.CPlusPlusModules && !LO.CurrentModule.empty()) {
+
+    ASTReader *R = getCurrentASTUnit().getASTReader().get();
+    unsigned SubModuleCount = R->getTotalNumSubmodules();
+    serialization::ModuleFile &MF = R->getModuleManager().getPrimaryModule();
+    Out << "  ====== C++20 Module structure ======\n";
+
+    if (MF.ModuleName != LO.CurrentModule)
+      Out << "  Mismatched module names : " << MF.ModuleName << " and "
+          << LO.CurrentModule << "\n";
+
+    struct SubModInfo {
+      unsigned Idx;
+      Module *Mod;
+      Module::ModuleKind Kind;
+      std::string &Name;
+      bool Seen;
+    };
+    std::map<std::string, SubModInfo> SubModMap;
+    auto PrintSubMapEntry = [&](std::string Name, Module::ModuleKind Kind) {
+      Out << "    " << ModuleKindName(Kind) << " '" << Name << "'";
+      auto I = SubModMap.find(Name);
+      if (I == SubModMap.end())
+        Out << " was not found in the sub modules!\n";
+      else {
+        I->second.Seen = true;
+        Out << " is at index #" << I->second.Idx << "\n";
+      }
+    };
+    Module *Primary = nullptr;
+    for (unsigned Idx = 0; Idx <= SubModuleCount; ++Idx) {
+      Module *M = R->getModule(Idx);
+      if (!M)
+        continue;
+      if (M->Name == LO.CurrentModule) {
+        Primary = M;
+        Out << "  " << ModuleKindName(M->Kind) << " '" << LO.CurrentModule
+            << "' is the Primary Module at index #" << Idx << "\n";
+        SubModMap.insert({M->Name, {Idx, M, M->Kind, M->Name, true}});
+      } else
+        SubModMap.insert({M->Name, {Idx, M, M->Kind, M->Name, false}});
+    }
+    if (Primary) {
+      if (!Primary->submodules().empty())
+        Out << "   Sub Modules:\n";
+      for (auto MI : Primary->submodules()) {
+        PrintSubMapEntry(MI->Name, MI->Kind);
+      }
+      if (!Primary->Imports.empty())
+        Out << "   Imports:\n";
+      for (auto IMP : Primary->Imports) {
+        PrintSubMapEntry(IMP->Name, IMP->Kind);
+      }
+      if (!Primary->Exports.empty())
+        Out << "   Exports:\n";
+      for (unsigned MN = 0, N = Primary->Exports.size(); MN != N; ++MN) {
+        if (Module *M = Primary->Exports[MN].getPointer()) {
+          PrintSubMapEntry(M->Name, M->Kind);
+        }
+      }
+    }
+    // Now let's print out any modules we did not see as part of the Primary.
+    for (auto SM : SubModMap) {
+      if (!SM.second.Seen && SM.second.Mod) {
+        Out << "  " << ModuleKindName(SM.second.Kind) << " '" << SM.first
+            << "' at index #" << SM.second.Idx
+            << " has no direct reference in the Primary\n";
+      }
+    }
+    Out << "  ====== ======\n";
+  }
+
+  // The reminder of the output is produced from the listener as the AST
+  // FileCcontrolBlock is (re-)parsed.
   ASTReader::readASTFileControlBlock(
       getCurrentFile(), FileMgr, getCompilerInstance().getPCHContainerReader(),
       /*FindModuleFileExtensions=*/true, Listener,
       HSOpts.ModulesValidateDiagnosticOptions);
 }
 
 //===----------------------------------------------------------------------===//
 // Preprocessor Actions

clang/test/Modules/cxx20-module-file-info.cpp

+// Test output from -module-file-info about C++20 modules.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/mod-info-tu1.cpp \
+// RUN:  -o %t/A.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info %t/A.pcm | FileCheck \
+// RUN:  --check-prefix=CHECK-A %s
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/mod-info-tu2.cpp \
+// RUN:  -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info %t/B.pcm | FileCheck \
+// RUN:  --check-prefix=CHECK-B %s
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/mod-info-tu3.cpp \
+// RUN:  -fmodule-file=%t/A.pcm -fmodule-file=%t/B.pcm -o %t/Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info %t/Foo.pcm | FileCheck \
+// RUN:  --check-prefix=CHECK-FOO %s
+
+// expected-no-diagnostics
+
+//--- mod-info-tu1.cpp
+export module A;
+
+void a();
+
+// CHECK-A: ====== C++20
+// CHECK-A-NEXT: Interface Unit 'A' is the Primary Module at index #1
+
+//--- mod-info-tu2.cpp
+export module B;
+
+void b();
+
+// CHECK-B: ====== C++20
+// CHECK-B-NEXT: Interface Unit 'B' is the Primary Module at index #1
+
+//--- mod-info-tu3.cpp
+module;
+
+export module Foo;
+
+import A;
+export import B;
+
+namespace hello {
+export void say(const char *);
+}
+
+void foo() {}
+
+// CHECK-FOO: ====== C++20
+// CHECK-FOO-NEXT:  Interface Unit 'Foo' is the Primary Module at index #3
+// CHECK-FOO-NEXT:   Sub Modules:
+// CHECK-FOO-NEXT:    Global Module Fragment '<global>' is at index #4
+// CHECK-FOO-NEXT:   Imports:
+// CHECK-FOO-NEXT:    Interface Unit 'A' is at index #1
+// CHECK-FOO-NEXT:   Exports:
+// CHECK-FOO-NEXT:    Interface Unit 'B' is at index #2