Improve diagnostics for missing import / #include of module.
Fix a few bugs where we would fail to properly determine header to
module correspondence when determining whether to suggest a #include or
import, and suggest a #include more often in language modes where there
is no import syntax. Generally, if the target is in a header with
include guards or #pragma once, we should suggest either #including or
importing that header, and not importing a module that happens to
textually include it.
In passing, improve the notes we attach to the corresponding
diagnostics: calling an entity that we couldn't see "previous" is
confusing.
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index d8041ae..03bd36b 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -646,24 +646,8 @@
}
const FileEntry *
-Preprocessor::getModuleHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
- Module *M,
- SourceLocation Loc) {
- assert(M && "no module to include");
-
- // If the context is the global module fragment of some module, we never
- // want to return that file; instead, we want the innermost include-guarded
- // header that it included.
- bool InGlobalModuleFragment = M->Kind == Module::GlobalModuleFragment;
-
- // If we have a module import syntax, we shouldn't include a header to
- // make a particular module visible.
- if ((getLangOpts().ObjC || getLangOpts().CPlusPlusModules ||
- getLangOpts().ModulesTS) &&
- !InGlobalModuleFragment)
- return nullptr;
-
- Module *TopM = M->getTopLevelModule();
+Preprocessor::getHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
+ SourceLocation Loc) {
Module *IncM = getModuleForLocation(IncLoc);
// Walk up through the include stack, looking through textual headers of M
@@ -677,37 +661,50 @@
if (!FE)
break;
- if (InGlobalModuleFragment) {
- if (getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE))
- return FE;
- Loc = SM.getIncludeLoc(ID);
- continue;
- }
+ // We want to find all possible modules that might contain this header, so
+ // search all enclosing directories for module maps and load them.
+ HeaderInfo.hasModuleMap(FE->getName(), /*Root*/ nullptr,
+ SourceMgr.isInSystemHeader(Loc));
- bool InTextualHeader = false;
- for (auto Header : HeaderInfo.getModuleMap().findAllModulesForHeader(FE)) {
- if (!Header.getModule()->isSubModuleOf(TopM))
- continue;
-
- if (!(Header.getRole() & ModuleMap::TextualHeader)) {
- // If this is an accessible, non-textual header of M's top-level module
- // that transitively includes the given location and makes the
- // corresponding module visible, this is the thing to #include.
- if (Header.isAccessibleFrom(IncM))
- return FE;
-
+ bool InPrivateHeader = false;
+ for (auto Header : HeaderInfo.findAllModulesForHeader(FE)) {
+ if (!Header.isAccessibleFrom(IncM)) {
// It's in a private header; we can't #include it.
// FIXME: If there's a public header in some module that re-exports it,
// then we could suggest including that, but it's not clear that's the
// expected way to make this entity visible.
+ InPrivateHeader = true;
continue;
}
- InTextualHeader = true;
+ // We'll suggest including textual headers below if they're
+ // include-guarded.
+ if (Header.getRole() & ModuleMap::TextualHeader)
+ continue;
+
+ // If we have a module import syntax, we shouldn't include a header to
+ // make a particular module visible. Let the caller know they should
+ // suggest an import instead.
+ if (getLangOpts().ObjC || getLangOpts().CPlusPlusModules ||
+ getLangOpts().ModulesTS)
+ return nullptr;
+
+ // If this is an accessible, non-textual header of M's top-level module
+ // that transitively includes the given location and makes the
+ // corresponding module visible, this is the thing to #include.
+ return FE;
}
- if (!InTextualHeader)
- break;
+ // FIXME: If we're bailing out due to a private header, we shouldn't suggest
+ // an import either.
+ if (InPrivateHeader)
+ return nullptr;
+
+ // If the header is includable and has an include guard, assume the
+ // intended way to expose its contents is by #include, not by importing a
+ // module that transitively includes it.
+ if (getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE))
+ return FE;
Loc = SM.getIncludeLoc(ID);
}