[PATCH] Add -fno-modules-implicit-builds.

Richard Smith richard at metafoo.co.uk
Wed Dec 17 11:39:08 PST 2014


Your change and your documentation don't match. The difference is the behavior in the case where a module is not provided explicitly but is found in the module cache; your documentation says this is invalid but your code accepts it. I think the more principled approach would be to do as your documentation suggests: require all modules to be provided via explicitly-specified `.pcm` files, and never look in the module cache.

I would also like to bikeshed the flag name; `-fno-implicit-modules` seems like a better name to me. I'd also suggest renaming `-fno-modules-implicit-maps` to `-fno-implicit-module-maps` because "module map" is the term of art here, and it doesn't make sense to split this term. Grouping these flags under `-fmodules` seems secondary to having them make sense as flag names.


================
Comment at: docs/Modules.rst:213-214
@@ -212,1 +212,4 @@
 
+``-fno-modules-implicit-bulids``
+  All modules used by the build must be specified with ``-fmodule-file``.
+
----------------
Please also add documentation for `-fmodule-file` if you're going to reference it from here. =)

================
Comment at: include/clang/Basic/DiagnosticCommonKinds.td:84-85
@@ -83,2 +83,4 @@
 def err_module_not_built : Error<"could not build module '%0'">, DefaultFatal;
+def err_module_build_disabled: Error<
+  "module '%0' not found, but module build was disabled">, DefaultFatal;
 def err_module_lock_failure : Error<
----------------
This seems like the wrong message to produce -- the problem here is that the module *was* found in some loaded module map file, but a prebuilt form of that module was not provided.

http://reviews.llvm.org/D6707

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list