A few comments, but generally this looks really good.
================ Comment at: include/clang/Basic/LangOptions.h:84 @@ +83,3 @@ + /// \brief The module for which the translation unit is a source. + std::string ModuleSourceFor; + ---------------- We have LangOptions::CurrentModule, set by -fmodule-name=<name>. Is this so different that we need a separate value/option? ================ Comment at: docs/Modules.rst:198 @@ +197,3 @@ +``-fmodules-indirect-check`` + Extend checking of module imports and includes to indirect uses of modules. + ---------------- I wonder... do we have to have this as an option, or can we simply say that there is one right answer and implement only that? ================ Comment at: docs/Modules.rst:195 @@ +194,3 @@ +``-fmodules-source-for=module-id`` + Consider a source file as a part of the given module. + ---------------- How is this different from -fmodule-name=module-id? ================ Comment at: lib/Basic/Module.cpp:271 @@ +270,3 @@ + } + // FIXME: Add handling of wildcard module specifications. +} ---------------- Why do we even have wildcards for uses? It seems that one would have to enumerate all of the uses separately for this feature to be useful. ================ Comment at: lib/Lex/PPDirectives.cpp:613 @@ +612,3 @@ + violatesUseDeclarations(RequestingModule, RequestedModule)) + Diag(FilenameLoc, diag::error_undeclared_use_of_module) + << Filename; ---------------- I guess we don't get Fix-Its within module map files... or do we? ================ Comment at: lib/Serialization/ASTReader.cpp:3032 @@ -3031,1 +3031,3 @@ + + //FIXME: How do we load the 'use'd modules? They may not be submodules. ---------------- You can read/write global "submodule IDs". The "sub" is misleading in this case; it means that one might be referring to either a top-level module or to a submodule. ================ Comment at: lib/Serialization/ASTWriter.cpp:2426 @@ -2425,1 +2425,3 @@ + //FIXME: How do we emit the 'use'd modules? They may not be submodules. + ---------------- Same comment as above; you can use getSubmoduleID() to refer to any module, whether it is a top-level module or a submodule. http://llvm-reviews.chandlerc.com/D1546 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
