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

Richard Smith richard at metafoo.co.uk
Wed Jan 14 19:19:36 PST 2015


================
Comment at: include/clang/Basic/DiagnosticCommonKinds.td:84-86
@@ -83,2 +83,5 @@
 def err_module_not_built : Error<"could not build module '%0'">, DefaultFatal;
+def err_module_build_disabled: Error<
+  "module '%0' needs to be compiled, but implicit module compilation was "
+  "disabled">, DefaultFatal;
 def err_module_lock_failure : Error<
----------------
This diagnostic suggests that it'd be OK if the module didn't need to be compiled. Maybe something like "module '%0' is needed but has not been provided, and implicit use of module files is disabled"?

================
Comment at: include/clang/Driver/Options.td:684-689
@@ -683,2 +683,8 @@
   Group<f_Group>, Flags<[DriverOption, CC1Option]>;
+def fimplicit_modules :
+  Flag <["-"], "fimplicit-modules">,
+  Group<f_Group>, Flags<[DriverOption, CC1Option]>;
+def fno_implicit_modules :
+  Flag <["-"], "fno-implicit-modules">,
+  Group<f_Group>, Flags<[DriverOption, CC1Option]>;
 def fretain_comments_from_system_headers : Flag<["-"], "fretain-comments-from-system-headers">, Group<f_Group>, Flags<[CC1Option]>;
----------------
It doesn't look like you're forwarding these from the driver to the frontend (and you have no test coverage for the driver flags). The same appears to be true of `-fmodules-implicit-maps`, which currently doesn't seem to work as a driver option...

Also, it's conventional to only provide one of the `-fblah` and `-fno-blah options` as a `-cc1` option (the non-default flag only); see `OPT_fmodules_decluse` for an example.

================
Comment at: test/Modules/no-implicit-builds.cpp:3-6
@@ +2,6 @@
+
+// Produce an error if a module is needed, but not found.
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodules-cache-path=%t -fmodule-name=a \
+// RUN:     -fmodule-map-file=%S/Inputs/no-implicit-builds/a.modulemap \
+// RUN:     -fno-implicit-modules %s -verify
+
----------------
Do you need the `-fmodule-name=` in these tests? It doesn't seem relevant to the subject matter. If you can remove that, can you also remove a.modulemap entirely and directly use b.modulemap here?

================
Comment at: test/Modules/no-implicit-builds.cpp:26-27
@@ +25,4 @@
+// RUN:     -emit-module %S/Inputs/no-implicit-builds/b.modulemap -Rmodule-build \
+// RUN:     -fno-implicit-modules 2>&1 | FileCheck --check-prefix=CHECK-BUILD --allow-empty %s
+// CHECK-BUILD-NOT: {{building module 'b'}}
+//
----------------
Is this CHECK useful?

http://reviews.llvm.org/D6707

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






More information about the cfe-commits mailing list