[Modules] Add ability to specify module name to module file mapping in a file

[boris]

Summary

Add the -fmodule-file-map=[<prefix>=]<file> option which can be used to specify a file that contains module name to precompiled modules files mapping, similar to -fmodule-file=<name>=<file>. The <prefix> can be used to only consider certain lines which can be useful if we want to store the mapping in an already existing file (for example, as comments in the .d makefile fragment generated with the -M option or some such).

Additional notes:

1. Based on this mailing list discussion: http://lists.llvm.org/pipermail/cfe-dev/2017-June/054431.html

2. Based on the functionality implemented in: https://reviews.llvm.org/D35020

[dblaikie]

Couple of drive by comments - no doubt Richard will have a more in depth review, but figured this might save a round or two.

lib/Frontend/CompilerInvocation.cpp

1561–1565

Run the whole change through clang-format (there are tools to help do this over a patch or git revision range) so it matches LLVM coding conventions

1576

Prefer copy init over direct init:

for (size_t B = 0, E = 0;

& probably != rather than < is more canonical/common in the LLVM codebase.

1586

There's probably a StringRef-y way to do this rather than going back through raw pointers? (StringRef::substr or the like)

[boris]

David, thanks for the review. Uploading the new revision (also rebased on HEAD).

lib/Frontend/CompilerInvocation.cpp

1576

All done except the != change -- B can actually go one over size (see the npos case in the loop body).

[boris]

New patch revision with David's comments addressed.

[boris]

Ping.

[boris]

Ping.

[bruno]

Hi Boris,

This is a handy option, very nice. Can you also add testcases?

lib/Frontend/CompilerInvocation.cpp

1561

auto Buf -> auto *Buf

[rsmith]

I wonder whether this feature really pulls its weight compared to using @file and prepending -fmodule-file= to each line.

The big difference between this patch and an @file seems to be the filtering by prefix / storing this information as a subset of the data in a file. Can you say a bit more about how you're envisioning this integrating into build systems, such that that would be a natural way to model this mapping?

[boris]

Yes, the main "feature" of this approach compared to @file is the ability to reuse an already existing file to store this information. Most build systems that support C/C++ compilation have to store auxiliary dependency information at least for the extracted header dependencies (those .d files generated by the -M option family, for example) but some also store hashes of options, compiler version/signature, etc. So instead of creating a yet another file (per translation unit), the idea is to reuse the already existing one by storing the mapping with some "distinguishing" prefix. As a concrete example, a make-based build system could append it to the .d file (which is a makefile fragment) as comments. How exactly this information is extracted is still an open question but I think this approach is generic enough to accommodate a wide range of possibilities (for example, -M could produce this information or the build system could append it itself after the -M is done).

[boris]

New revision this time with the tests (which got dropped from the earlier revision diff for some reason).

[boris]

Another attempt to upload a clean diff (also rebased on latest HEAD).

[boris]

Ping.

[boris]

Ping.

[boris]

Ping.

[boris]

Ping.

[boris]

Ping.

[boris]

Ping.

[rsmith]

I'm still unconvinced that this mechanism is worth supporting.

The use case of putting this information into .d files doesn't make sense to me. .d files are generally outputs of prior compilations, whereas this information is a compilation input, needed in every compilation (including the first), and must be eagerly updated if a change to the source code adds a new dependency.

[boris]

Hi Richard,

Thanks for still entertaining the idea. Yes, I believe, in order to support modules (TS) the build system will have to extract module (and header while at it) dependency information prior to compilation rather than as a byproduct of compilation (which is how most build systems do it now). I've written on how all this can work (as well as the support that would be required from the compiler) here:

https://build2.org/article/cxx-modules-misconceptions.xhtml#build

FWIW, we do it this way in build2 (and already have a .d file that we would like to reuse) and get 3x speedup using Clang on a modularized build. Also Gaby told me that he is working on a tool for VC that will do extraction of header and module dependency information (as a single preprocessor pass).

[boris]

Ping.

Also, to add to my previous comment, even if for a moment we ignore the header dependencies and when they are extracted, a modern build system normally tracks other kinds of what can be called "auxiliary dependency information": some form of compiler signature, hash of options that were used to compile the file, etc., so that when any of those change, the object file gets recompiled automatically. For example, in build2, we store all these signatures/hashes plus the header and module dependency information in a single .d file (which we call auxiliary dependency database). What I am trying to show by this is that it is well established that for C/C++ compilation there has to be an extra file for each .o where such information is stored. And it seems natural to want to reuse this file for supplying the "module map" to the compiler instead of creating yet another per-.o file.

docs/Modules.rst

``-fmodule-file=[<name>=]<file>``
   Specify the mapping of module names to precompiled module files. If the
   name is omitted, then the module file is loaded whether actually required
   or not. If the name is specified, then the mapping is treated as another
   prebuilt module search mechanism (in addition to ``-fprebuilt-module-path``)
   and the module is only loaded if required. Note that in this case the
   specified file also overrides this module's paths that might be embedded
   in other precompiled module files.
 
+``-fmodule-file-map=[<prefix>=]<file>``
+  Read the mapping of module names to precompiled module files from file. If
+  the argument includes optional prefix, then only lines starting with this
+  string are considered (with the prefix itself ignored). Each mapping entry
+  should be in the same form as the ``-fmodule-file`` value with module
+  name. Leading and trailing whitespaces in the value as well as blank lines
+  are ignored. The line prefix can be used to store the mapping in an already
+  existing file, for example, as comments in makefile fragments produced by
+  the ``-M`` option family.
+
 ``-fprebuilt-module-path=<directory>``
   Specify the path to the prebuilt modules. If specified, we will look for modules in this directory for a given top-level module name. We don't need a module map for loading prebuilt modules in this directory and the compiler will not try to rebuild these modules. This can be specified multiple times.
 
 Module Semantics
 ================
 
 Modules are modeled as if each submodule were a separate translation unit, and a module import makes names from the other translation unit visible. Each submodule starts with a new preprocessor state and an empty translation unit.
 

include/clang/Basic/DiagnosticDriverKinds.td

 
 def err_drv_unsupported_linker : Error<"unsupported value '%0' for -linker option">;
 def err_drv_defsym_invalid_format : Error<"defsym must be of the form: sym=value: %0">;
 def err_drv_defsym_invalid_symval : Error<"Value is not an integer: %0">;
 def warn_drv_msvc_not_found : Warning<
   "unable to find a Visual Studio installation; "
   "try running Clang from a developer command prompt">,
   InGroup<DiagGroup<"msvc-not-found">>;
+
+def err_drv_invalid_module_file_map : Error<"invalid module file mapping '%0'">;
 }

include/clang/Driver/Options.td

 def fmodule_implementation_of : Separate<["-"], "fmodule-implementation-of">,
   Flags<[CC1Option]>, Alias<fmodule_name_EQ>;
 def fmodule_map_file : Joined<["-"], "fmodule-map-file=">,
   Group<f_Group>, Flags<[DriverOption,CC1Option]>, MetaVarName<"<file>">,
   HelpText<"Load this module map file">;
 def fmodule_file : Joined<["-"], "fmodule-file=">,
   Group<i_Group>, Flags<[DriverOption,CC1Option]>, MetaVarName<"[<name>=]<file>">,
   HelpText<"Specify the mapping of module name to precompiled module file, or load a module file if name is omitted.">;
+def fmodule_file_map : Joined<["-"], "fmodule-file-map=">,
+  Group<i_Group>, Flags<[DriverOption,CC1Option]>, MetaVarName<"[<prefix>=]<file>">,
+  HelpText<"Read the mapping of module names to precompiled module files from <file>.">;
 def fmodules_ignore_macro : Joined<["-"], "fmodules-ignore-macro=">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Ignore the definition of the given macro when building and loading modules">;
 def fmodules_decluse : Flag <["-"], "fmodules-decluse">, Group<f_Group>,
   Flags<[DriverOption,CC1Option]>,
   HelpText<"Require declaration of modules used within a module">;
 def fmodules_strict_decluse : Flag <["-"], "fmodules-strict-decluse">, Group<f_Group>,
   Flags<[DriverOption,CC1Option]>,
   HelpText<"Like -fmodules-decluse but requires all headers to be in modules">;

lib/Driver/ToolChains/Clang.cpp

static void RenderModulesOptions(Compilation &C, const Driver &D,
   // names to precompiled module files (the module is loaded only if used).
   // The -fmodule-file=<file> form can be used to unconditionally load
   // precompiled module files (whether used or not).
   if (HaveModules)
     Args.AddAllArgs(CmdArgs, options::OPT_fmodule_file);
   else
     Args.ClaimAllArgs(options::OPT_fmodule_file);
 
+  // -fmodule-file-map specifies the file containing the module names to
+  // precompiled module files mapping.
+  //
+  if (HaveModules)
+    Args.AddAllArgs(CmdArgs, options::OPT_fmodule_file_map);
+  else
+    Args.ClaimAllArgs(options::OPT_fmodule_file_map);
+
   // When building modules and generating crashdumps, we need to dump a module
   // dependency VFS alongside the output.
   if (HaveClangModules && C.isForDiagnostics()) {
     SmallString<128> VFSDir(Output.getFilename());
     llvm::sys::path::replace_extension(VFSDir, ".cache");
     // Add the cache directory as a temp so the crash diagnostics pick it up.
     C.addTempFile(Args.MakeArgString(VFSDir));
 

lib/Frontend/CompilerInvocation.cpp

if (ClangResourceDir != "")
     llvm::sys::path::append(P, ClangResourceDir);
   else
     llvm::sys::path::append(P, "..", Twine("lib") + CLANG_LIBDIR_SUFFIX,
                             "clang", CLANG_VERSION_STRING);
 
   return P.str();
 }
 
+// Read the mapping of module names to precompiled module files from a file.
+// The argument can include an optional line prefix ([<prefix>]=<file>), in
+// which case only lines that start with the prefix are considered (with the
+// prefix and the following whitespaces, if any, ignored).
+//
+// Each mapping entry should be in the same form as the -fmodule-file option
+// value (<name>=<file>) with leading/trailing whitespaces ignored.
+//
+static void LoadModuleFileMap(HeaderSearchOptions &Opts,
+                              DiagnosticsEngine &Diags, FileManager &FileMgr,
+                              StringRef Val, const std::string &Arg) {
+  // See if we have the prefix.
+  StringRef File;
+  StringRef Prefix;
+  if (Val.find('=') != StringRef::npos) {
+    auto Pair = Val.split('=');
+    Prefix = Pair.first;
+    File = Pair.second;
+    if (Prefix.empty()) {
+      Diags.Report(diag::err_drv_invalid_value) << Arg << Val;
+      return;
+    }
+  } else
+    File = Val;
+
+  if (File.empty()) {
+    Diags.Report(diag::err_drv_invalid_value) << Arg << Val;
+    return;
+  }
+
+  auto Buf = FileMgr.getBufferForFile(File);
+  if (!Buf) {
+    Diags.Report(diag::err_cannot_open_file)
+        << File << Buf.getError().message();
+    return;
+  }
+
+  // Read the file line by line.
+  StringRef Str = Buf.get()->getBuffer();
+  for (size_t B = 0, E = 0; B < Str.size(); B = E + 1) {
+    E = Str.find_first_of(StringRef("\n\0", 2), B);
+
+    if (E == StringRef::npos)
+      E = Str.size();
+    else if (Str[E] == '\0')
+      break; // The file (or the rest of it) is binary, bail out.
+
+    // [B, E) is our line. Compare and skip the prefix, if any.
+    StringRef Line = Str.substr(B, E - B);
+    if (!Prefix.empty()) {
+      if (!Line.startswith(Prefix))
+        continue;
+
+      Line = Line.substr(Prefix.size());
+    }
+
+    // Skip leading and trailing whitespaces and ignore blanks (even if they
+    // had prefix; think make comments).
+    Line = Line.trim();
+    if (Line.empty())
+      continue;
+
+    if (Line.find('=') == StringRef::npos) {
+      Diags.Report(diag::err_drv_invalid_module_file_map) << Line;
+      continue;
+    }
+
+    Opts.PrebuiltModuleFiles.insert(Line.split('='));
+  }
+}
+
 static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
-                                  const std::string &WorkingDir) {
+                                  DiagnosticsEngine &Diags,
+                                  FileManager &FileMgr) {
   using namespace options;
   Opts.Sysroot = Args.getLastArgValue(OPT_isysroot, "/");
   Opts.Verbose = Args.hasArg(OPT_v);
   Opts.UseBuiltinIncludes = !Args.hasArg(OPT_nobuiltininc);
   Opts.UseStandardSystemIncludes = !Args.hasArg(OPT_nostdsysteminc);
   Opts.UseStandardCXXIncludes = !Args.hasArg(OPT_nostdincxx);
   if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
     Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
   Opts.ResourceDir = Args.getLastArgValue(OPT_resource_dir);
 
   // Canonicalize -fmodules-cache-path before storing it.
   SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
   if (!(P.empty() || llvm::sys::path::is_absolute(P))) {
+    const std::string &WorkingDir (FileMgr.getFileSystemOpts().WorkingDir);
     if (WorkingDir.empty())
       llvm::sys::fs::make_absolute(P);
     else
       llvm::sys::fs::make_absolute(WorkingDir, P);
   }
   llvm::sys::path::remove_dots(P);
   Opts.ModuleCachePath = P.str();
 
   Opts.ModuleUserBuildPath = Args.getLastArgValue(OPT_fmodules_user_build_path);
   // Only the -fmodule-file=<name>=<file> form.
   for (const Arg *A : Args.filtered(OPT_fmodule_file)) {
     StringRef Val = A->getValue();
     if (Val.find('=') != StringRef::npos)
       Opts.PrebuiltModuleFiles.insert(Val.split('='));
   }
+  for (const Arg *A : Args.filtered(OPT_fmodule_file_map))
+    LoadModuleFileMap(Opts, Diags, FileMgr, A->getValue(), A->getAsString(Args));
   for (const Arg *A : Args.filtered(OPT_fprebuilt_module_path))
     Opts.AddPrebuiltModulePath(A->getValue());
   Opts.DisableModuleHash = Args.hasArg(OPT_fdisable_module_hash);
   Opts.ModulesHashContent = Args.hasArg(OPT_fmodules_hash_content);
   Opts.ModulesValidateDiagnosticOptions =
       !Args.hasArg(OPT_fmodules_disable_diagnostic_validation);
   Opts.ImplicitModuleMaps = Args.hasArg(OPT_fimplicit_module_maps);
   Opts.ModuleMapFileHomeIsCwd = Args.hasArg(OPT_fmodule_map_file_home_is_cwd);
static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
   case frontend::RewriteMacros:
   case frontend::RunPreprocessorOnly:
     return true;
   }
   llvm_unreachable("invalid frontend action");
 }
 
 static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
-                                  FileManager &FileMgr,
                                   DiagnosticsEngine &Diags,
                                   frontend::ActionKind Action) {
   using namespace options;
   Opts.ImplicitPCHInclude = Args.getLastArgValue(OPT_include_pch);
   Opts.ImplicitPTHInclude = Args.getLastArgValue(OPT_include_pth);
   if (const Arg *A = Args.getLastArg(OPT_token_cache))
       Opts.TokenCache = A->getValue();
   else
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
   Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
   Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
   ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
   Success &=
       ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
                           false /*DefaultDiagColor*/, false /*DefaultShowOpt*/);
   ParseCommentArgs(LangOpts.CommentOpts, Args);
   ParseFileSystemArgs(Res.getFileSystemOpts(), Args);
+
+  // File manager used during option parsing (e.g., for loading map files,
+  // etc).
+  //
+  FileManager FileMgr(Res.getFileSystemOpts());
+
   // FIXME: We shouldn't have to pass the DashX option around here
   InputKind DashX = ParseFrontendArgs(Res.getFrontendOpts(), Args, Diags,
                                       LangOpts.IsHeaderFile);
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
                               Res.getTargetOpts());
-  ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args,
-                        Res.getFileSystemOpts().WorkingDir);
+  ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags, FileMgr);
   if (DashX.getFormat() == InputKind::Precompiled ||
       DashX.getLanguage() == InputKind::LLVM_IR) {
     // ObjCAAutoRefCount and Sanitize LangOpts are used to setup the
     // PassManager in BackendUtil.cpp. They need to be initializd no matter
     // what the input type is.
     if (Args.hasArg(OPT_fobjc_arc))
       LangOpts.ObjCAutoRefCount = 1;
     // PIClevel and PIELevel are needed during code generation and this should be
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
 
   // FIXME: Override value name discarding when asan or msan is used because the
   // backend passes depend on the name of the alloca in order to print out
   // names.
   Res.getCodeGenOpts().DiscardValueNames &=
       !LangOpts.Sanitize.has(SanitizerKind::Address) &&
       !LangOpts.Sanitize.has(SanitizerKind::Memory);
 
-  // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents of
-  // PCH file and find the original header name. Remove the need to do that in
-  // ParsePreprocessorArgs and remove the FileManager
-  // parameters from the function and the "FileManager.h" #include.
-  FileManager FileMgr(Res.getFileSystemOpts());
-  ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+  ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
                         Res.getFrontendOpts().ProgramAction);
   ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
                               Res.getFrontendOpts().ProgramAction);
 
   // Turn on -Wspir-compat for SPIR target.
   llvm::Triple T(Res.getTargetOpts().Triple);
   auto Arch = T.getArch();
   if (Arch == llvm::Triple::spir || Arch == llvm::Triple::spir64) {