[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