[PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

[Boris Kolpackov via Phabricator via cfe-commits]

boris created this revision.

Extend the -fmodule-file option to support the [<name>=]<file> value format.
If the name is omitted, then the old semantics is preserved (the module file
is loaded whether needed or not). If the name is specified, then the mapping
is treated as just another prebuilt module search mechanism, similar to
-fprebuilt-module-path, and the module file is only loaded if actually
used (e.g., via import). With one exception: this mapping also overrides
module file references embedded in other modules (which can be useful if
module files are moved/renamed as often happens during remote compilation).

This override semantics requires some extra work: we now store the module
name in addition to the file name in the serialized AST representation as
well as keep track of already loaded prebuilt modules by-name in addition
to by-file.

Patch by Boris Kolpackov

---------------

Additional notes:

1. Based on this mailing list discussion:

  http://lists.llvm.org/pipermail/cfe-dev/2017-June/054431.html

2. The need to change the serialized AST representation was a bit more than what I hoped for my first patch but based on this FIXME comment I recon this is moving in right direction:

  ASTReader::ReadModuleOffsetMap():

  // FIXME: Looking up dependency modules by filename is horrible.

  So now, at least for prebuilt modules, we look up by the module name.

3. Overloading -fmodule-file for this admittedly fairly different semantics might look like a bad idea and source of some unnecessary complexity. The problem with a separate option approach is the difficulty of finding a decent name that is not already used (e.g., -fmodule-map is out because of -fmodule-map-file; more details in the mailing list thread). But I am still open to changing this to a separate option if there are strong feelings (and good name suggestions ;-)).

4. I plan to implement a companion option that will read this mapping from a file. I will submit it as a separate patch once the general validity of the approach is confirmed.


https://reviews.llvm.org/D35020

Files:
  docs/ClangCommandLineReference.rst
  docs/Modules.rst
  include/clang/Driver/Options.td
  include/clang/Lex/HeaderSearch.h
  include/clang/Lex/HeaderSearchOptions.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ModuleManager.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/GlobalModuleIndex.cpp
  lib/Serialization/ModuleManager.cpp
  test/CXX/modules-ts/basic/basic.search/module-import.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35020.105299.patch
Type: text/x-patch
Size: 25964 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170705/4c400d46/attachment-0001.bin>