[PATCH] Module use declarations (II)

Doug Gregor dgregor at apple.com
Mon Sep 2 22:22:46 PDT 2013


  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



More information about the cfe-commits mailing list