[modules] PR20507: Avoid silent textual inclusion.

[silvas]

Summary

This pretty naively follows the implementation suggested by Richard.

I'd like to switch these over to be fatal errors (since they are
essentially similar to err_pp_file_not_found), but I'm still investigating
some test failures that happen when I switched them over.

I'm really not a fan of the `M->isAvailable(getLangOpts(), getTargetInfo(),
Requirement, MissingHeader)` function; it seems to do too many things at
once, but for now I've done things in a sort of awkward way.

The changes to test/Modules/Inputs/declare-use/module.map
were necessitated because the thing that was meant to be tested there
(introduced in r197805) was predicated on silently falling back to textual
inclusion, which we no longer do.

The changes to test/Modules/Inputs/macro-reexport/module.modulemap
are just an overlooked missing header that seems to have been missing since
this code was committed (r213922), which is now caught.

[rsmith]

lib/Lex/ModuleMap.cpp

346–347 ↗

(On Diff #27609)

Should we also teach isBetterKnownHeader to consider an available module to be better than an unavailable one? (If I have two modules covering a header file, and one of them requires "blah" and the other requires "!blah", we should presumably pick the available module.)

[silvas]

lib/Lex/ModuleMap.cpp

346–347 ↗

(On Diff #27609)

I'm having a hard time thinking of a legitimate use for that, or in general to ever have two modules referencing the same file. Do you have something in particular in mind?

But that does raise the larger question is how to make this insensitive to whatever order we are iterating here (which I assume is determined in some relatively fragile way by the order of command line arguments or order in the module map file). From a user's perspective, it is extremely jarring to have the compiler's behavior depend in a fragile way on such things (like the unix linker order-dependence fiasco). I would prefer to just have a hard error if two modules reference the same header, thus we wouldn't even be iterating here at all, and users will always be immediately alerted to errors.

[benlangmuir]

include/clang/Basic/DiagnosticCommonKinds.td

89–90 ↗

(On Diff #27609)

When this diag was only used for @import it was okay because you get a separate diagnostic telling you that the module build failed, so it was clear what happened.

@import Foo; error: header 'bar.h' not found
error: could not build module 'Foo'

But now it's odd to get an error about a different header than the one you included with no indication that it's because of a module:

#include <foo.h> // error: header 'bar.h' not found

We could shoehorn the module name into the diagnostic, or perhaps emit a note on the header decl in the module map file.

[benlangmuir]

include/clang/Basic/DiagnosticCommonKinds.td

89–90 ↗

(On Diff #27609)

Err, reading fail on my part, you're already emitting this at the header decl location. Maybe we should have a note for the module import location though.

[silvas]

Add note about location where implicit import happened.

Any ideas on how to better explain/word it would be appreciated.

[benlangmuir]

include/clang/Basic/DiagnosticLexKinds.td

626 ↗

(On Diff #27809)

I'd just drop "submodule of top-level ".

[silvas]

include/clang/Basic/DiagnosticLexKinds.td

626 ↗

(On Diff #27809)

I think it's an important qualification because the module that was in fact imported is not %0. %0 is the top-level module containing it, and the error is because some submodule of %0 (perhaps the one being imported, perhaps not) has the missing file.

From a user's perspective: "The header I included didn't ask for %0 to be imported. It asked for this submodule of it". Ideally we would be able to somehow communicate the "unity build" aspect of top-level modules for this to make sense, but I couldn't find a way to fit all that in a diagnostic.

Does that make sense?

[silvas]

Ping.

[benlangmuir]

From a user's perspective: "The header I included didn't ask for %0 to be imported. It asked for this submodule of it"

Alternatively, the user may not be aware of the internal structure of a top-level module and just expect it to work like it works with header includes :-) I'd still prefer the diagnostic without "submodule of top-level", which is also consistent with our "While building module" include-stack style notes. But I don't think this is worth holding up your patch over.

This question still seems to be outstanding:

So concretely for the present patch just the change to isBetterKnownHeader to take into account of isAvailable?

Richard, can you comment?

[benlangmuir]

I tried this patch out and found a few issues I've commented on inline. It also exposes the two issues below:

1. This patch exposes a compatibility issue with our 'Darwin' module map file (the module map that covers most of /usr/include on Darwin platforms).

error: module 'Darwin.C.excluded' requires feature 'excluded'

This "excluded" submodule was used as a hack for "assert.h" before the explicit support for "exclude" and "textual" headers were added to clang. After this patch, it won't be possible to include assert.h on Darwin, because the module it is in is missing a requirement. I think it would be very bad to break compatibility here, so I propose we add a compatibility hack to module map parsing that treats a module with a the requirement "excluded" as if all its headers were declared with "exclude". What do you think? I'm happy to implement this.

2. If a require decl is written *after* a header decl in a module, that header still gets treated as required. We should fix that before this is committed. E.g.

module Top {
  module A { header "foo.h" requires nope }
  module B { requires nope header "bar.h" }
}

We'll complain about missing foo.h, but not about bar.h.

lib/Lex/PPDirectives.cpp

1671 ↗

(On Diff #27809)

1. This uses MissingHeader.FileNameLoc, which we just proved is invalid :-)
2. We should also show diag::note_implicit_top_level_module_import_here in this diagnostic.

test/Modules/missing-header.cpp

13 ↗

(On Diff #27809)

We should have a test for including a header from a module with a missing requirement. And one where a missing header from a different submodule is okay because the submodule is missing a requirement.

[rsmith]

In D10423#198214, @benlangmuir wrote:

1. This patch exposes a compatibility issue with our 'Darwin' module map file (the module map that covers most of /usr/include on Darwin platforms).

error: module 'Darwin.C.excluded' requires feature 'excluded'

This "excluded" submodule was used as a hack for "assert.h" before the explicit support for "exclude" and "textual" headers were added to clang. After this patch, it won't be possible to include assert.h on Darwin, because the module it is in is missing a requirement. I think it would be very bad to break compatibility here, so I propose we add a compatibility hack to module map parsing that treats a module with a the requirement "excluded" as if all its headers were declared with "exclude". What do you think? I'm happy to implement this.

Should this be textual instead of exclude? That's generally a better way to handle assert.h, I think. (That said, I don't think it really matters, because no-one cares which module assert.h is in, and we don't yet build PTH for textual headers.)

So concretely for the present patch just the change to isBetterKnownHeader to take into account of isAvailable?

Yes, I think so.

[benlangmuir]

Should this be textual instead of exclude? That's generally a better way to handle assert.h, I think. (That said, I don't think it really matters, because no-one cares which module assert.h is in, and we don't yet build PTH for textual headers.)

Yeah, assert.h should really be textual. I just thought it seemed odd that marking something requires excluded would correspond to textual instead of exclude. I think either would be fine, really. Also, we'd probably want a warning about this behaviour.

And... I just discovered another module map bug this change exposed. We mislabeled the IOKit.avc submodule as requires cplusplus. After this patch, the IOKit module (and therefore all of Cocoa) won't build outside C++ modes :-( I propose an egregious special case hack that checks for the offending module by name and drops the bogus requirement.

[rsmith]

I'm fine with special-case hacks to keep the current Darwin module maps working, so long as they're relatively self-contained and very unlikely to trigger in any other cases. (We do equally ugly things to parse broken libstdc++ headers.)

I agree that exclude makes more sense for requires excluded as a "supported" feature, but if this is another egregious hack for Darwin, I don't think it matters which we choose. (Maybe we should also check the module name here?)

[silvas]

Ben:

• Added better source location.
• Updated testing. I renamed to auto-import-unavailable.cpp and expanded testing as you asked for. Before, I was piggybacking on missing-header.m and related files, but since that is a different code path (and has different requirements and diagnostics), I thought it was better to keep the test separate and self-contained.
• also emit note_implicit_top_level_module_import_here on missing requirement.

Richard:

• available-is-better.cpp and the changes to isBetterKnownHeader to take into account availability.

Does this look okay?

Ben: hopefully the present patch is complete enough for you to start
figuring out exactly what hacks to add and where.

[benlangmuir]

I agree that exclude makes more sense for requires excluded as a "supported" feature, but if this is another egregious hack for Darwin, I don't think it matters which we choose.

Sure, I'm just as happy to use textual, since every case I've seen that's the more correct behavior.

(Maybe we should also check the module name here?)

I found another use of this pattern in our Tcl module. I suspect when I look back at older versions of the SDK I'm going to find more of these cases. How about we don't limit this hack to specific modules, but we add a warning to say it's treating these as "textual".

[benlangmuir]

• Added better source location.

It should really be the requirement location, but I guess we don't store that anywhere. This can be improved separately.

• Updated testing. I renamed to auto-import-unavailable.cpp and expanded testing as you asked for. Before, I was piggybacking on missing-header.m and related files, but since that is a different code path (and has different requirements and diagnostics), I thought it was better to keep the test separate and self-contained.
• also emit note_implicit_top_level_module_import_here on missing requirement.

Changes look good, thanks.

I replied to your email about the issue when a "requires" clause comes *after* a header declaration. I didn't actually hit that problem in the wild - only in a synthetic test - so maybe it can be addressed separately. If you and Richard agree, then from my perspective your patch LGTM and we just need to get the Darwin hacks in before it lands.

Ben: hopefully the present patch is complete enough for you to start
figuring out exactly what hacks to add and where.

Definitely. I can work from this, thanks.

[rsmith]

lib/Lex/PPDirectives.cpp

1665 ↗

(On Diff #29234)

I think this whole block should be moved down to line 1761 or so:

1. The code currently ensures that it always calls InclusionDirective on the callback object for every #include, even if that include fails.
2. We don't yet know that the file is actually part of SuggestedModule; it could be in the directory of an umbrella header whose module is unavailable, but it might not be part of that module.
3. If we somehow got a suggested module but no file, we should not produce additional spurious diagnostics beyond the "file not found" diagnostic we already produced.

test/Modules/Inputs/auto-import-unavailable/module.modulemap

9 ↗

(On Diff #29234)

Please add some content to the empty files (even if just a comment) or this test will misbehave on content-addressed file systems, where all the empty files will be treated as the same file.

[rsmith]

lib/Lex/PPDirectives.cpp

1665 ↗

(On Diff #29234)

One other thing: we should only diagnose when ShouldEnter is false. If we're entering the file anyway, we don't need the module to be available; only those files within it that we're actually entering need to be present.

[rsmith]

lib/Lex/PPDirectives.cpp

1665 ↗

(On Diff #29234)

Hmm, that's not quite right (ShouldEnter will be false the second time we reach a header with an include guard too). I think the right thing is: don't diagnose this if the suggested module's top-level module is either the current module or the "implementation of" module (that is, if it's a module we'll /always/ enter textually).

[silvas]

• markUnavailable.patch686 BDownload

[silvas]

• msg-9622-116.txt154 BDownload

[silvas]

• msg-17067-308.txt154 BDownload

[benlangmuir]

Something weird seems to have been going on with Phab emails and I missed your patch for the header/requires ordering issue. Anyway, I fixed it in r242055 with a similar approach.

[silvas]

Updating based on Richard's feedback.

Richard, is this something like you imagined?

The PPCallbacks is still not being called, like the existing branch
// We hit an error processing the import. Bail out.. The fix will be
similar for both, so this at least isn't making it any harder to fix the
situation.

[rsmith]

lib/Lex/PPDirectives.cpp

1680–1698 ↗

(On Diff #30196)

The call to loadModule a few lines below already does this. Is this change necessary?

[silvas]

lib/Lex/PPDirectives.cpp

1680–1698 ↗

(On Diff #30196)

I think it is. We have more information available here and can diagnose the situation better. Ideally the call to loadModule here would take a Module * and could assert the module to be available.

[rsmith]

lib/Lex/PPDirectives.cpp

1680–1698 ↗

(On Diff #30196)

OK, that sounds like a good general direction. I don't like the duplication here, but if the plan is to eventually remove it, I'm OK with that.

I'm still concerned about the incomplete umbrella header case, but that really seems like a fundamental design problem with implicit module generation for incomplete umbrella headers -- if we can't build the umbrella header, we can't tell whether a file is part of it, so if the umbrella header module is unavailable, we have to either reject all inclusions of headers in its directory or silently ignore the umbrella header (and silently ignoring modules is exactly what you're preventing with this change).

[benlangmuir]

Since Richard accepted this, I'll pipe up that I have a patch adding the hacks needed to make this work on Darwin. I need to add some tests and then I'll put it up for review - hopefully tomorrow.

[rsmith]

Yeah, I should have said: please wait for Ben to get the Darwin fixes ready before committing :)

[silvas]

In D10423#208693, @benlangmuir wrote:

Since Richard accepted this, I'll pipe up that I have a patch adding the hacks needed to make this work on Darwin. I need to add some tests and then I'll put it up for review - hopefully tomorrow.

Awesome, thanks! I'll hold off on committing until we have a working set of workarounds ready to roll.

[benlangmuir]

Darwin module map hacks: http://reviews.llvm.org/D11403
We also need to fix clang's _Builting_Intrinsics.arm module. I posted a thread about this to cfe-dev for feedback: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-July/044225.html

cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td

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' is needed but has not been provided, and implicit use of module "
   "files is disabled">, DefaultFatal;
+def err_module_unavailable : Error<
+  "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
+def err_module_header_missing : Error<
+  "%select{|umbrella }0header '%1' not found">;
 def err_module_lock_failure : Error<
   "could not acquire lock file for module '%0'">, DefaultFatal;
 def err_module_lock_timeout : Error<
   "timed out waiting to 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<

cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td

def err_missing_module_name : Error<
   DefaultFatal;
 def err_missing_module : Error<
   "no module named '%0' declared in module map file '%1'">, DefaultFatal;
 def err_no_submodule : Error<"no submodule named %0 in module '%1'">;
 def err_no_submodule_suggest : Error<
   "no submodule named %0 in module '%1'; did you mean '%2'?">;
 def warn_missing_submodule : Warning<"missing submodule '%0'">,
   InGroup<IncompleteUmbrella>;
-def err_module_unavailable : Error<
-  "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
-def err_module_header_missing : Error<
-  "%select{|umbrella }0header '%1' not found">;
 def err_module_cannot_create_includes : Error<
   "cannot create includes file for module %0: %1">;
 def warn_module_config_macro_undef : Warning<
   "%select{definition|#undef}0 of configuration macro '%1' has no effect on "
   "the import of '%2'; pass '%select{-D%1=...|-U%1}0' on the command line "
   "to configure the module">,
   InGroup<ConfigMacros>;
 def note_module_def_undef_here : Note<

cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td

 def err_mmap_expected_feature : Error<"expected a feature name">;
 def err_mmap_expected_attribute : Error<"expected an attribute name">;
 def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">,
   InGroup<IgnoredAttributes>;
 
 def warn_auto_module_import : Warning<
   "treating #%select{include|import|include_next|__include_macros}0 as an "
   "import of module '%1'">, InGroup<AutoImport>, DefaultIgnore;
+def note_implicit_top_level_module_import_here : Note<
+  "submodule of top-level module '%0' implicitly imported here">;
 def warn_uncovered_module_header : Warning<
   "umbrella header for module '%0' does not include header '%1'">, 
   InGroup<IncompleteUmbrella>;
 def err_expected_id_building_module : Error<
   "expected a module name in '__building_module' expression">;
 def warn_use_of_private_header_outside_module : Warning<
   "use of private header from outside its module: '%0'">,
   InGroup<DiagGroup<"private-header">>, DefaultError;

cfe/trunk/lib/Lex/ModuleMap.cpp

diag::kind DiagID = RequestingModule->getTopLevelModule()->IsFramework ?
         diag::warn_non_modular_include_in_framework_module :
         diag::warn_non_modular_include_in_module;
     Diags.Report(FilenameLoc, DiagID) << RequestingModule->getFullModuleName();
   }
 }
 
 static bool isBetterKnownHeader(const ModuleMap::KnownHeader &New,
                                 const ModuleMap::KnownHeader &Old) {
+  // Prefer available modules.
+  if (New.getModule()->isAvailable() && !Old.getModule()->isAvailable())
+    return true;
+
   // Prefer a public header over a private header.
   if ((New.getRole() & ModuleMap::PrivateHeader) !=
       (Old.getRole() & ModuleMap::PrivateHeader))
     return !(New.getRole() & ModuleMap::PrivateHeader);
 
   // Prefer a non-textual header over a textual header.
   if ((New.getRole() & ModuleMap::TextualHeader) !=
       (Old.getRole() & ModuleMap::TextualHeader))
ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File) {
   HeadersMap::iterator Known = findKnownHeader(File);
   if (Known != Headers.end()) {
     ModuleMap::KnownHeader Result;
     // Iterate over all modules that 'File' is part of to find the best fit.
     for (KnownHeader &H : Known->second) {
       // Prefer a header from the current module over all others.
       if (H.getModule()->getTopLevelModule() == CompilingModule)
         return MakeResult(H);
-      // Cannot use a module if it is unavailable.
-      if (!H.getModule()->isAvailable())
-        continue;
       if (!Result || isBetterKnownHeader(H, Result))
         Result = H;
     }
     return MakeResult(Result);
   }
 
   SmallVector<const DirectoryEntry *, 2> SkippedDirs;
   KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs);

cfe/trunk/lib/Lex/PPDirectives.cpp

#endif
   // Determine whether we should try to import the module for this #include, if
   // there is one. Don't do so if precompiled module support is disabled or we
   // are processing this module textually (because we're building the module).
   if (File && SuggestedModule && getLangOpts().Modules &&
       SuggestedModule.getModule()->getTopLevelModuleName() !=
           getLangOpts().CurrentModule &&
       SuggestedModule.getModule()->getTopLevelModuleName() !=
           getLangOpts().ImplementationOfModule) {
+
+    // If this include corresponds to a module but that module is
+    // unavailable, diagnose the situation and bail out.
+    if (!SuggestedModule.getModule()->isAvailable()) {
+      clang::Module::Requirement Requirement;
+      clang::Module::UnresolvedHeaderDirective MissingHeader;
+      Module *M = SuggestedModule.getModule();
+      // Identify the cause.
+      (void)M->isAvailable(getLangOpts(), getTargetInfo(), Requirement,
+                           MissingHeader);
+      if (MissingHeader.FileNameLoc.isValid()) {
+        Diag(MissingHeader.FileNameLoc, diag::err_module_header_missing)
+            << MissingHeader.IsUmbrella << MissingHeader.FileName;
+      } else {
+        Diag(M->DefinitionLoc, diag::err_module_unavailable)
+            << M->getFullModuleName() << Requirement.second << Requirement.first;
+      }
+      Diag(FilenameTok.getLocation(),
+           diag::note_implicit_top_level_module_import_here)
+          << M->getTopLevelModuleName();
+      return;
+    }
+
     // Compute the module access path corresponding to this module.
     // FIXME: Should we have a second loadModule() overload to avoid this
     // extra lookup step?
     SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
     for (Module *Mod = SuggestedModule.getModule(); Mod; Mod = Mod->Parent)
       Path.push_back(std::make_pair(getIdentifierInfo(Mod->Name),
                                     FilenameTok.getLocation()));
     std::reverse(Path.begin(), Path.end());

cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h

+// missing_header/not_missing.h

cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_requirement.h

+// missing_requirement.h

cfe/trunk/test/Modules/Inputs/auto-import-unavailable/module.modulemap

+module missing_header {
+  module missing { header "missing_header/missing.h" }
+  module error_importing_this { header "missing_header/not_missing.h" }
+}
+
+module nonrequired_missing_header {
+  module unsatisfied_requires {
+    requires nonexistent_feature
+    header "nonrequired_missing_header/missing.h"
+  }
+  module fine_to_import {
+    header "nonrequired_missing_header/not_missing.h"
+  }
+}
+
+module missing_requirement {
+  requires nonexistent_feature
+  header "missing_requirement.h"
+}

cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h

+// nonrequired_missing_header/not_missing.h

cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h

+// nonrequired_missing_header/requires_feature_you_dont_have.h

cfe/trunk/test/Modules/Inputs/available-is-better/available-is-better.h

+#pragma once
+int available;

cfe/trunk/test/Modules/Inputs/available-is-better/module.modulemap

+// There is some order-dependence to how clang chooses modules, so make
+// sure that both the first and last modules here are ones that would
+// cause a test failure if they were picked.
+
+module unavailable_before {
+  requires nonexistent_feature
+  header "available-is-better.h"
+}
+
+module available {
+  header "available-is-better.h"
+}
+
+module unavailable_after {
+  requires nonexistent_feature
+  header "available-is-better.h"
+}

cfe/trunk/test/Modules/Inputs/declare-use/module.map

 module XD {
   header "d.h"
   use XA
   // Intentionally doesn't use XB to show that -fdecl-use isn't transitive.
 }
 
 module XE {
   header "e.h"
-  header "unavailable.h"
   use XA
   use XB
 }
 
 module XF {
   header "f.h"
-  header "unavailable.h"
   use XA
   use XB
 }
 
 module XG {
   header "g.h"
   header "g1.h"
   use XC

cfe/trunk/test/Modules/Inputs/macro-reexport/module.modulemap

module d {
   module d2 { header "d2.h" export * }
 }
 module e {
   module e1 { header "e1.h" export * }
   module e2 { header "e2.h" export * }
 }
 module f {
   module f1 { header "f1.h" export * }
-  module f2 { header "f2.h" export * }
 }

cfe/trunk/test/Modules/auto-import-unavailable.cpp

+// RUN: rm -rf %t
+// RUN: not %clang_cc1 -x c++ -Rmodule-build -DMISSING_HEADER -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=MISSING-HEADER
+// RUN: %clang_cc1 -x c++ -Rmodule-build -DNONREQUIRED_MISSING_HEADER -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=NONREQUIRED-MISSING-HEADER
+// RUN: not %clang_cc1 -x c++ -Rmodule-build -DMISSING_REQUIREMENT -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=MISSING-REQUIREMENT
+
+#ifdef MISSING_HEADER
+
+// Even if the header we ask for is not missing, if the top-level module
+// containing it has a missing header, then the whole top-level is
+// unavailable and we issue an error.
+
+// MISSING-HEADER: module.modulemap:2:27: error: header 'missing_header/missing.h' not found
+// MISSING-HEADER-DAG: auto-import-unavailable.cpp:[[@LINE+1]]:10: note: submodule of top-level module 'missing_header' implicitly imported here
+#include "missing_header/not_missing.h"
+
+// We should not attempt to build the module.
+// MISSING-HEADER-NOT: remark: building module
+
+#endif // #ifdef MISSING_HEADER
+
+
+#ifdef NONREQUIRED_MISSING_HEADER
+
+// However, if the missing header is dominated by an unsatisfied
+// `requires`, then that is acceptable.
+// This also tests that an unsatisfied `requires` elsewhere in the
+// top-level module doesn't affect an available module.
+
+// NONREQUIRED-MISSING-HEADER: auto-import-unavailable.cpp:[[@LINE+2]]:10: remark: building module 'nonrequired_missing_header'
+// NONREQUIRED-MISSING-HEADER: auto-import-unavailable.cpp:[[@LINE+1]]:10: remark: finished building module 'nonrequired_missing_header'
+#include "nonrequired_missing_header/not_missing.h"
+
+#endif // #ifdef NONREQUIRED_MISSING_HEADER
+
+
+#ifdef MISSING_REQUIREMENT
+
+// If the header is unavailable due to a missing requirement, an error
+// should be emitted if a user tries to include it.
+
+// MISSING-REQUIREMENT:module.modulemap:16:8: error: module 'missing_requirement' requires feature 'nonexistent_feature'
+// MISSING-REQUIREMENT: auto-import-unavailable.cpp:[[@LINE+1]]:10: note: submodule of top-level module 'missing_requirement' implicitly imported here
+#include "missing_requirement.h"
+
+// MISSING-REQUIREMENT-NOT: remark: building module
+
+#endif // #ifdef MISSING_REQUIREMENT

cfe/trunk/test/Modules/available-is-better.cpp

+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -Rmodule-build -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/available-is-better %s 2>&1 | FileCheck %s
+
+#include "available-is-better.h"
+// CHECK: remark: building module