Add -fno-modules-implicit-builds.

[klimek]

Summary

If this flag is set, we error out when a module build is required. This is
useful in environments where all required modules are passed via -fmodule-file.

[rsmith]

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.

docs/Modules.rst

213–214

Please also add documentation for -fmodule-file if you're going to reference it from here. =)

include/clang/Basic/DiagnosticCommonKinds.td

84–85

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.

[klimek]

Shed bikes, more docs, cop out earlier.

[klimek]

The only thing not done is the renaming of the modules-implicit-maps flag, which I'd like to do in a separate patch.

docs/Modules.rst

213–214

Done.

include/clang/Basic/DiagnosticCommonKinds.td

84–85

Better?

[rsmith]

Please add test coverage showing that we can use explicitly-specified modules via -fmodule-file=, and that we can't use implicitly-build modules that are in the cache.

lib/Frontend/CompilerInstance.cpp

1363–1367

This will also reject modules loaded via -fmodule-file=.

1375–1381

The check should go somewhere around here, after we've determined whether the user gave us a file containing this module. if (!Explicit && !ImplicitModules).

[klimek]

Added tests.

lib/Frontend/CompilerInstance.cpp

1363–1367

Oh, now I understand what "Explicit" means here...

1375–1381

Done.

[klimek]

Happy new Year ping (and sync).

[rsmith]

include/clang/Basic/DiagnosticCommonKinds.td

84–86

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"?

include/clang/Driver/Options.td

684–689

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.

test/Modules/no-implicit-builds.cpp

4–7

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?

27–28

Is this CHECK useful?

[klimek]

Apply review comments.

[klimek]

Remove whitespace diff.

include/clang/Basic/DiagnosticCommonKinds.td

84–86

Done.

include/clang/Driver/Options.td

684–689

Done.

test/Modules/no-implicit-builds.cpp

4–7

Done. I'm a little unsure whether the test now still tests everything I want it to test...

27–28

Done.

[rsmith]

docs/Modules.rst

216

-fmodule-file=<FILE> or similar would be clearer

lib/Driver/Tools.cpp

3912–3917 ↗

(On Diff #20075)

Maybe hold back on adding driver support for this flag until after it's renamed?

test/Modules/Inputs/no-implicit-builds/a.modulemap

2–6

This file appears to be unused.

test/Modules/no-implicit-builds.cpp

16–17

Add -Rmodule-build to the command line for safety / clarity, given this.

[klimek]

Review comments.

[klimek]

Whitespace

[klimek]

More whitespace.

[klimek]

Fix whitespace for real?

[klimek]

And one more whitespace cleanup.

docs/Modules.rst

216

Done, using <file> to match <directory> above.

lib/Driver/Tools.cpp

3912–3917 ↗

(On Diff #20075)

That is an excellent point, thx.

test/Modules/Inputs/no-implicit-builds/a.modulemap

2–6

Done.

test/Modules/no-implicit-builds.cpp

16–17

Done.

docs/Modules.rst

``-fmodule-map-file=<file>``
   Load the given module map file if a header from its directory or one of its subdirectories is loaded.
 
 ``-fmodules-search-all``
   If a symbol is not found, search modules referenced in the current module maps but not imported for symbols, so the error message can reference the module by name.  Note that if the global module index has not been built before, this might take some time as it needs to build all the modules.  Note that this option doesn't apply in module builds, to avoid the recursion.
 
 ``-fno-modules-implicit-maps``
   Suppresses the implicit search for files called ``module.modulemap`` and similar. Instead, module files need to be explicitly specified via ``-fmodule-map-file`` or transitively used.
 
+``-fno-implicit-modules``
+  All modules used by the build must be specified with ``-fmodule-file``.
+
+``-fmodule-file``
+  Load the given precompiled module file.
+
 Module Semantics
 ================
 
 Modules are modeled as if each submodule were a separate translation unit, and a module import makes names from the other translation unit visible. Each submodule starts with a new preprocessor state and an empty translation unit.
 
 .. note::
 
   This behavior is currently only approximated when building a module with submodules. Entities within a submodule that has already been built are visible when building later submodules in that module. This can lead to fragile modules that depend on the build order used for the submodules of the module, and should not be relied upon. This behavior is subject to change.

include/clang/Basic/DiagnosticCommonKinds.td

def warn_cxx98_compat_variadic_templates :
   Warning<"variadic templates are incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
 def err_default_special_members : Error<
   "only special member functions may be defaulted">;
 def err_deleted_non_function : Error<
   "only functions can have deleted definitions">;
 def err_module_not_found : Error<"module '%0' not found">, DefaultFatal;
 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<
   "could not acquire lock file for module '%0'">, DefaultFatal;
 def err_module_cycle : Error<"cyclic dependency in module '%0': %1">, 
   DefaultFatal;
 def note_pragma_entered_here : Note<"#pragma entered here">;  
 def note_decl_hiding_tag_type : Note<
   "%1 %0 is hidden by a non-type declaration of %0 here">;
 def err_attribute_not_type_attr : Error<

include/clang/Basic/LangOptions.def

 LANGOPT(MathErrno         , 1, 1, "errno support for math functions")
 BENIGN_LANGOPT(HeinousExtensions , 1, 0, "Extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules extension to C")
 LANGOPT(ModulesDeclUse    , 1, 0, "require declaration of module uses")
 LANGOPT(ModulesSearchAll  , 1, 1, "search even non-imported modules to find unresolved references")
 LANGOPT(ModulesStrictDeclUse, 1, 0, "require declaration of module uses and all headers to be in modules")
 LANGOPT(ModulesErrorRecovery, 1, 1, "automatically import modules as needed when performing error recovery")
 BENIGN_LANGOPT(ModulesImplicitMaps, 1, 1, "use files called module.modulemap implicitly as module maps")
+BENIGN_LANGOPT(ImplicitModules, 1, 1, "build modules that are not specified via -fmodule-file")
 COMPATIBLE_LANGOPT(Optimize          , 1, 0, "__OPTIMIZE__ predefined macro")
 COMPATIBLE_LANGOPT(OptimizeSize      , 1, 0, "__OPTIMIZE_SIZE__ predefined macro")
 LANGOPT(Static            , 1, 0, "__STATIC__ predefined macro (as opposed to __DYNAMIC__)")
 VALUE_LANGOPT(PackStruct  , 32, 0,
               "default struct packing maximum alignment")
 VALUE_LANGOPT(MaxTypeAlign  , 32, 0,
               "default maximum alignment for types")
 VALUE_LANGOPT(PICLevel    , 2, 0, "__PIC__ level")

 
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT
 #undef ENUM_LANGOPT
 #undef COMPATIBLE_ENUM_LANGOPT
 #undef BENIGN_ENUM_LANGOPT
 #undef VALUE_LANGOPT
-

include/clang/Driver/Options.td

 def fno_modules_search_all : Flag <["-"], "fno-modules-search-all">, Group<f_Group>,
   Flags<[DriverOption, CC1Option]>;
 def fmodules_implicit_maps :
   Flag <["-"], "fmodules-implicit-maps">,
   Group<f_Group>, Flags<[DriverOption, CC1Option]>;
 def fno_modules_implicit_maps :
   Flag <["-"], "fno-modules-implicit-maps">,
   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]>;
 
 def fmudflapth : Flag<["-"], "fmudflapth">, Group<f_Group>;
 def fmudflap : Flag<["-"], "fmudflap">, Group<f_Group>;
 def fnested_functions : Flag<["-"], "fnested-functions">, Group<f_Group>;
 def fnext_runtime : Flag<["-"], "fnext-runtime">, Group<f_Group>;
 def fno_access_control : Flag<["-"], "fno-access-control">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Disable C++ access control">;

lib/Frontend/CompilerInstance.cpp

CompilerInstance::loadModule(SourceLocation ImportLoc,
   if (Known != KnownModules.end()) {
     // Retrieve the cached top-level module.
     Module = Known->second;    
   } else if (ModuleName == getLangOpts().CurrentModule ||
              ModuleName == getLangOpts().ImplementationOfModule) {
     // This is the module we're building. 
     Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
     Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
   } else {
     // Search for a module with the given name.
     Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
     if (!Module) {
       getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
       << ModuleName
       << SourceRange(ImportLoc, ModuleNameLoc);
       ModuleBuildFailed = true;
       return ModuleLoadResult();
     }
 
     auto Override = ModuleFileOverrides.find(ModuleName);
     bool Explicit = Override != ModuleFileOverrides.end();
+    if (!Explicit && !getLangOpts().ImplicitModules) {
+      getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
+          << ModuleName;
+      ModuleBuildFailed = true;
+      return ModuleLoadResult();
+    }
 
     std::string ModuleFileName =
         Explicit ? Override->second
                  : PP->getHeaderSearchInfo().getModuleFileName(Module);
 
     // If we don't already have an ASTReader, create one now.
     if (!ModuleManager)
       createModuleManager();

lib/Frontend/CompilerInvocation.cpp

#include "clang/Frontend/LangStandards.def"
   Opts.ModulesDeclUse =
       Args.hasArg(OPT_fmodules_decluse) || Opts.ModulesStrictDeclUse;
   Opts.ModulesSearchAll = Opts.Modules &&
     !Args.hasArg(OPT_fno_modules_search_all) &&
     Args.hasArg(OPT_fmodules_search_all);
   Opts.ModulesErrorRecovery = !Args.hasArg(OPT_fno_modules_error_recovery);
   Opts.ModulesImplicitMaps = Args.hasFlag(OPT_fmodules_implicit_maps,
                                           OPT_fno_modules_implicit_maps, true);
+  Opts.ImplicitModules =
+      Args.hasFlag(OPT_fimplicit_modules, OPT_fno_implicit_modules, true);
   Opts.CharIsSigned = Opts.OpenCL || !Args.hasArg(OPT_fno_signed_char);
   Opts.WChar = Opts.CPlusPlus && !Args.hasArg(OPT_fno_wchar);
   Opts.ShortWChar = Args.hasFlag(OPT_fshort_wchar, OPT_fno_short_wchar, false);
   Opts.ShortEnums = Args.hasArg(OPT_fshort_enums);
   Opts.Freestanding = Args.hasArg(OPT_ffreestanding);
   Opts.NoBuiltin = Args.hasArg(OPT_fno_builtin) || Opts.Freestanding;
   Opts.NoMathBuiltin = Args.hasArg(OPT_fno_math_builtin);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);

test/Modules/Inputs/no-implicit-builds/a.modulemap

+module "a" {
+  use "b"
+}
+extern module "b" "b.modulemap"
 No newline at end of file

test/Modules/Inputs/no-implicit-builds/b.h

+#ifndef B_H
+#define B_H
+
+int b;
+
+#endif

test/Modules/Inputs/no-implicit-builds/b.modulemap

+module "b" {
+  header "b.h"
+}

test/Modules/no-implicit-builds.cpp

+// RUN: rm -rf %t
+
+// 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
+
+// Compile the module and put it into the cache.
+// 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:     %s -Rmodule-build 2>&1 | FileCheck --check-prefix=CHECK-CACHE-BUILD %s
+// CHECK-CACHE-BUILD: {{building module 'b'}}
+
+// Produce an error if a module is found in the cache but implicit modules is off.
+// Note that the command line must match the command line for the first check, otherwise
+// this check might not find the module in the cache and trivially succeed.
+// 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
+
+// Verify that we can still pass the module via -fmodule-file when implicit modules
+// are switched off:
+// - First, explicitly compile the module:
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=b -o %t/b.pcm \
+// 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'}}
+//
+// - Next, verify that we can load it:
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=a -fmodule-file=%t/b.pcm \
+// RUN:     -fmodule-map-file=%S/Inputs/no-implicit-builds/a.modulemap \
+// RUN:     -fno-implicit-modules %s
+
+#include "Inputs/no-implicit-builds/b.h"  // expected-error {{needs to be compiled}}