[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

boris added a comment. > For example, my experimental support for P1689 > is at: [...]. The implementation looks > relatively trivial to me. The simplicity is pretty important. Two points: 1. It's worth considering the simplicity of the overall system

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

boris added a comment. Just to add a few points on the module mapper discussion: overall, the current view is that there are two ways to handle C++20 modules from the build system's POV: (1) pre-scan the codebase to figure out what is what and who depends on whom ahead of compilation and (2)

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

boris added a comment. I don't think it will be wise for me to accept this revision since I can't claim to have good understanding of Clang's internal modules model. I think it will be wise to have Richard take a look. Comment at: lib/Basic/Module.cpp:349 + // Everything

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

boris added a comment. LGTM. Maybe it makes sense to also test that an unrelated translation unit that imports module 'test' sees neither 'a' nor 'b'. https://reviews.llvm.org/D40443 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40270: [Modules TS] Added re-export support

boris added a comment. All our tests pass as well. Thanks for your work! Repository: rL LLVM https://reviews.llvm.org/D40270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40270: [Modules TS] Added re-export support

boris added a comment. LGTM. Will be happy to also run our re-export tests in build2 once this is merged. https://reviews.llvm.org/D40270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

boris added a comment. 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,

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

boris added a comment. 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

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris updated this revision to Diff 116442. boris added a comment. Another attempt to upload a clean diff (also rebased on latest HEAD). https://reviews.llvm.org/D37299 Files: docs/Modules.rst include/clang/Basic/DiagnosticDriverKinds.td include/clang/Driver/Options.td

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

boris updated this revision to Diff 116438. boris marked an inline comment as done. boris added a comment. New revision this time with the tests (which got dropped from the earlier revision diff for some reason). https://reviews.llvm.org/D37299 Files: docs/Modules.rst

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

boris added a comment. 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

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D37299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris updated this revision to Diff 113779. boris marked an inline comment as done. boris added a comment. New patch revision with David's comments addressed. https://reviews.llvm.org/D37299 Files: docs/Modules.rst include/clang/Basic/DiagnosticDriverKinds.td

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

boris marked 3 inline comments as done. boris added a comment. David, thanks for the review. Uploading the new revision (also rebased on HEAD). Comment at: lib/Frontend/CompilerInvocation.cpp:1576 + StringRef Str = Buf.get()->getBuffer(); + for (size_t B(0), E(B); B <

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

boris created this revision. Add the -fmodule-file-map=[=] option which can be used to specify a file that contains module name to precompiled modules files mapping, similar to -fmodule-file==. The can be used to only consider certain lines which can be useful if we want to store the mapping

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

boris marked 2 inline comments as done. boris added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:986 +if (Val.find('=') == StringRef::npos) + Opts.ExtraDeps.push_back(Val); + } rsmith wrote: > Does a module file specified via

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

boris added a comment. Ping. https://reviews.llvm.org/D35020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added inline comments. Comment at: lib/Serialization/ASTReader.cpp:4145-4146 +// by-name lookup. +if (Type == MK_PrebuiltModule || Type == MK_ExplicitModule) + ModuleMgr.registerPrebuilt(F); break; rsmith wrote: > I'm

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

boris marked 6 inline comments as done. boris added a comment. I've marked as "done" items that I have resolved in my local revision (not yet uploaded) and have added one comment for further feedback. Comment at: lib/Frontend/CompilerInvocation.cpp:982 +StringRef Val =

[PATCH] D35678: Omit sumbodule semantics for TS modules

boris added a comment. In https://reviews.llvm.org/D35678#842891, @rsmith wrote: > I'd still like the id flattening moved to the caller. [...] I'm fine with > that being done as a separate change after this one, though, if you'd prefer. Yes, that would probably be easier. > Do you need

[PATCH] D35678: Omit sumbodule semantics for TS modules

boris updated this revision to Diff 110875. boris added a comment. Richard, sorry for the last ping, somehow I missed your review. I've uploaded a new revision with a test case (note that the issue is with 'module a.b' not 'import a.b' but I have tested both for good measure). My understanding

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

boris added a comment. Ping. https://reviews.llvm.org/D35020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35678: Omit sumbodule semantics for TS modules

boris added a comment. Ping. https://reviews.llvm.org/D35678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35678: Omit sumbodule semantics for TS modules

boris added a comment. Ping. https://reviews.llvm.org/D35678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. https://reviews.llvm.org/D35020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35678: Omit sumbodule semantics for TS modules

boris added a comment. Ping. https://reviews.llvm.org/D35678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris added a comment. Ping. FWIW, I went ahead and implemented this functionality in GCC. It has been merged into its c++-modules branch. https://reviews.llvm.org/D35020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35678: Omit sumbodule semantics for TS modules

boris created this revision. If a TS module name has more than one component (e.g., foo.bar) then we erroneously activate the submodule semantics when encountering a module declaration in the module implementation unit (e.g., module foo.bar;). https://reviews.llvm.org/D35678 Files:

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

boris added a comment. Ping. I am holding off on proposing the same functionality to GCC because I want to make sure the command line interface is the same for both compilers (GCC has less baggage in this area, option-name-wise). So confirming that at least the naming/semantics are acceptable

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

boris updated this revision to Diff 107017. boris added a comment. Rebase on latest HEAD. https://reviews.llvm.org/D35020 Files: docs/ClangCommandLineReference.rst docs/Modules.rst include/clang/Driver/Options.td include/clang/Lex/HeaderSearch.h

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

boris added a comment. Ping. https://reviews.llvm.org/D35020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

boris updated this revision to Diff 106041. boris added a comment. Rebase on latest HEAD. https://reviews.llvm.org/D35020 Files: docs/ClangCommandLineReference.rst docs/Modules.rst include/clang/Driver/Options.td include/clang/Lex/HeaderSearch.h

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

boris created this revision. Extend the -fmodule-file option to support the [=] 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