r197805 - Enable layering check in unavailable modules.
Daniel Jasper
djasper at google.com
Fri Dec 20 04:09:37 PST 2013
Author: djasper
Date: Fri Dec 20 06:09:36 2013
New Revision: 197805
URL: http://llvm.org/viewvc/llvm-project?rev=197805&view=rev
Log:
Enable layering check in unavailable modules.
If a header file belonging to a certain module is not found on the
filesystem, that header gets marked as unavailable. Now, the layering
warning (-fmodules-decluse) should still warn about headers of this
module being wrongfully included. Currently, headers belonging to those
modules are just treated as not belonging to modules at all which means
they can be included freely from everywhere.
To implement this (somewhat) cleanly, I have moved most of the layering
checks into the ModuleMap. This will also help with showing FixIts
later.
Modified:
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Modules/Inputs/declare-use/module.map
Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=197805&r1=197804&r2=197805&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Fri Dec 20 06:09:36 2013
@@ -175,6 +175,13 @@ private:
/// resolved.
Module *resolveModuleId(const ModuleId &Id, Module *Mod, bool Complain) const;
+ /// \brief Looks up the modules that \p File corresponds to.
+ ///
+ /// If \p File represents a builtin header within Clang's builtin include
+ /// directory, this also loads all of the module maps to see if it will get
+ /// associated with a specific module (e.g. in /usr/include).
+ HeadersMap::iterator findKnownHeader(const FileEntry *File);
+
public:
/// \brief Construct a new module map.
///
@@ -212,15 +219,24 @@ public:
/// used from. Used to disambiguate if a header is present in multiple
/// modules.
///
- /// \param FoundInModule If not null will be set to \c true if the header was
- /// found in non-exclude header declaration of a module.
- ///
/// \returns The module KnownHeader, which provides the module that owns the
/// given header file. The KnownHeader is default constructed to indicate
/// that no module owns this header file.
KnownHeader findModuleForHeader(const FileEntry *File,
- Module *RequestingModule = NULL,
- bool *FoundInModule = NULL);
+ Module *RequestingModule = NULL);
+
+ /// \brief Reports errors if a module must not include a specific file.
+ ///
+ /// \param RequestingModule The module including a file.
+ ///
+ /// \param FilenameLoc The location of the inclusion's filename.
+ ///
+ /// \param Filename The included filename as written.
+ ///
+ /// \param File The included file.
+ void diagnoseHeaderInclusion(Module *RequestingModule,
+ SourceLocation FilenameLoc, StringRef Filename,
+ const FileEntry *File);
/// \brief Determine whether the given header is part of a module
/// marked 'unavailable'.
Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=197805&r1=197804&r2=197805&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Fri Dec 20 06:09:36 2013
@@ -1506,25 +1506,6 @@ private:
/// points to.
Module *getModuleForLocation(SourceLocation FilenameLoc);
- /// \brief Verify that a private header is included only from within its
- /// module.
- bool violatesPrivateInclude(Module *RequestingModule,
- const FileEntry *IncFileEnt,
- ModuleMap::ModuleHeaderRole Role,
- Module *RequestedModule);
-
- /// \brief Verify that a module includes headers only from modules that it
- /// has declared that it uses.
- bool violatesUseDeclarations(Module *RequestingModule,
- Module *RequestedModule);
-
- /// \brief Verify that it is legal for the source file that \p FilenameLoc
- /// points to to include the file \p Filename.
- ///
- /// Tries to reuse \p IncFileEnt.
- void verifyModuleInclude(SourceLocation FilenameLoc, StringRef Filename,
- const FileEntry *IncFileEnt);
-
// Macro handling.
void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterTopLevelIfndef);
void HandleUndefDirective(Token &Tok);
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=197805&r1=197804&r2=197805&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Dec 20 06:09:36 2013
@@ -159,21 +159,115 @@ static bool isBuiltinHeader(StringRef Fi
.Default(false);
}
-ModuleMap::KnownHeader
-ModuleMap::findModuleForHeader(const FileEntry *File,
- Module *RequestingModule,
- bool *FoundInModule) {
+ModuleMap::HeadersMap::iterator
+ModuleMap::findKnownHeader(const FileEntry *File) {
HeadersMap::iterator Known = Headers.find(File);
-
- // If we've found a builtin header within Clang's builtin include directory,
- // load all of the module maps to see if it will get associated with a
- // specific module (e.g., in /usr/include).
if (Known == Headers.end() && File->getDir() == BuiltinIncludeDir &&
isBuiltinHeader(llvm::sys::path::filename(File->getName()))) {
HeaderInfo.loadTopLevelSystemModules();
- Known = Headers.find(File);
+ return Headers.find(File);
+ }
+ return Known;
+}
+
+// Returns 'true' if 'RequestingModule directly uses 'RequestedModule'.
+static bool directlyUses(const Module *RequestingModule,
+ const Module *RequestedModule) {
+ return std::find(RequestingModule->DirectUses.begin(),
+ RequestingModule->DirectUses.end(),
+ RequestedModule) != RequestingModule->DirectUses.end();
+}
+
+static bool violatesPrivateInclude(Module *RequestingModule,
+ const FileEntry *IncFileEnt,
+ ModuleMap::ModuleHeaderRole Role,
+ Module *RequestedModule) {
+ #ifndef NDEBUG
+ // Check for consistency between the module header role
+ // as obtained from the lookup and as obtained from the module.
+ // This check is not cheap, so enable it only for debugging.
+ SmallVectorImpl<const FileEntry *> &PvtHdrs
+ = RequestedModule->PrivateHeaders;
+ SmallVectorImpl<const FileEntry *>::iterator Look
+ = std::find(PvtHdrs.begin(), PvtHdrs.end(), IncFileEnt);
+ bool IsPrivate = Look != PvtHdrs.end();
+ assert((IsPrivate && Role == ModuleMap::PrivateHeader)
+ || (!IsPrivate && Role != ModuleMap::PrivateHeader));
+ #endif
+ return Role == ModuleMap::PrivateHeader &&
+ RequestedModule->getTopLevelModule() != RequestingModule;
+}
+
+void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
+ SourceLocation FilenameLoc,
+ StringRef Filename,
+ const FileEntry *File) {
+ // No errors for indirect modules. This may be a bit of a problem for modules
+ // with no source files.
+ if (RequestingModule != SourceModule)
+ return;
+
+ if (RequestingModule)
+ resolveUses(RequestingModule, /*Complain=*/false);
+
+ HeadersMap::iterator Known = findKnownHeader(File);
+ if (Known == Headers.end())
+ return;
+
+ Module *Private = NULL;
+ Module *NotUsed = NULL;
+ for (SmallVectorImpl<KnownHeader>::iterator I = Known->second.begin(),
+ E = Known->second.end();
+ I != E; ++I) {
+ // Excluded headers don't really belong to a module.
+ if (I->getRole() == ModuleMap::ExcludedHeader)
+ continue;
+
+ // If 'File' is part of 'RequestingModule' we can definitely include it.
+ if (I->getModule() == RequestingModule)
+ return;
+
+ // Remember private headers for later printing of a diagnostic.
+ if (violatesPrivateInclude(RequestingModule, File, I->getRole(),
+ I->getModule())) {
+ Private = I->getModule();
+ continue;
+ }
+
+ // If uses need to be specified explicitly, we are only allowed to return
+ // modules that are explicitly used by the requesting module.
+ if (RequestingModule && LangOpts.ModulesDeclUse &&
+ !directlyUses(RequestingModule, I->getModule())) {
+ NotUsed = I->getModule();
+ continue;
+ }
+
+ // We have found a module that we can happily use.
+ return;
}
+ // We have found a header, but it is private.
+ if (Private != NULL) {
+ Diags.Report(FilenameLoc, diag::error_use_of_private_header_outside_module)
+ << Filename;
+ return;
+ }
+
+ // We have found a module, but we don't use it.
+ if (NotUsed != NULL) {
+ Diags.Report(FilenameLoc, diag::error_undeclared_use_of_module)
+ << RequestingModule->getFullModuleName() << Filename;
+ return;
+ }
+
+ // Headers for which we have not found a module are fine to include.
+}
+
+ModuleMap::KnownHeader
+ModuleMap::findModuleForHeader(const FileEntry *File,
+ Module *RequestingModule) {
+ HeadersMap::iterator Known = findKnownHeader(File);
+
if (Known != Headers.end()) {
ModuleMap::KnownHeader Result = KnownHeader();
@@ -185,9 +279,6 @@ ModuleMap::findModuleForHeader(const Fil
if (I->getRole() == ModuleMap::ExcludedHeader)
continue;
- if (FoundInModule)
- *FoundInModule = true;
-
// Cannot use a module if it is unavailable.
if (!I->getModule()->isAvailable())
continue;
@@ -200,9 +291,7 @@ ModuleMap::findModuleForHeader(const Fil
// If uses need to be specified explicitly, we are only allowed to return
// modules that are explicitly used by the requesting module.
if (RequestingModule && LangOpts.ModulesDeclUse &&
- std::find(RequestingModule->DirectUses.begin(),
- RequestingModule->DirectUses.end(),
- I->getModule()) == RequestingModule->DirectUses.end())
+ !directlyUses(RequestingModule, I->getModule()))
continue;
Result = *I;
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=197805&r1=197804&r2=197805&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri Dec 20 06:09:36 2013
@@ -550,73 +550,6 @@ Module *Preprocessor::getModuleForLocati
}
}
-bool Preprocessor::violatesPrivateInclude(
- Module *RequestingModule,
- const FileEntry *IncFileEnt,
- ModuleMap::ModuleHeaderRole Role,
- Module *RequestedModule) {
- #ifndef NDEBUG
- // Check for consistency between the module header role
- // as obtained from the lookup and as obtained from the module.
- // This check is not cheap, so enable it only for debugging.
- SmallVectorImpl<const FileEntry *> &PvtHdrs
- = RequestedModule->PrivateHeaders;
- SmallVectorImpl<const FileEntry *>::iterator Look
- = std::find(PvtHdrs.begin(), PvtHdrs.end(), IncFileEnt);
- bool IsPrivate = Look != PvtHdrs.end();
- assert((IsPrivate && Role == ModuleMap::PrivateHeader)
- || (!IsPrivate && Role != ModuleMap::PrivateHeader));
- #endif
- return Role == ModuleMap::PrivateHeader &&
- RequestedModule->getTopLevelModule() != RequestingModule;
-}
-
-bool Preprocessor::violatesUseDeclarations(
- Module *RequestingModule,
- Module *RequestedModule) {
- ModuleMap &ModMap = HeaderInfo.getModuleMap();
- ModMap.resolveUses(RequestingModule, /*Complain=*/false);
- const SmallVectorImpl<Module *> &AllowedUses = RequestingModule->DirectUses;
- SmallVectorImpl<Module *>::const_iterator Declared =
- std::find(AllowedUses.begin(), AllowedUses.end(), RequestedModule);
- return Declared == AllowedUses.end();
-}
-
-void Preprocessor::verifyModuleInclude(SourceLocation FilenameLoc,
- StringRef Filename,
- const FileEntry *IncFileEnt) {
- Module *RequestingModule = getModuleForLocation(FilenameLoc);
- if (RequestingModule)
- HeaderInfo.getModuleMap().resolveUses(RequestingModule, /*Complain=*/false);
- bool FoundInModule = false;
- ModuleMap::KnownHeader RequestedModule =
- HeaderInfo.getModuleMap().findModuleForHeader(
- IncFileEnt, RequestingModule, &FoundInModule);
-
- if (!FoundInModule)
- return; // The header is not part of a module.
-
- if (RequestingModule == RequestedModule.getModule())
- return; // No faults wihin a module, or between files both not in modules.
-
- if (RequestingModule != HeaderInfo.getModuleMap().SourceModule)
- return; // No errors for indirect modules.
- // This may be a bit of a problem for modules with no source files.
-
- if (RequestedModule && violatesPrivateInclude(RequestingModule, IncFileEnt,
- RequestedModule.getRole(),
- RequestedModule.getModule()))
- Diag(FilenameLoc, diag::error_use_of_private_header_outside_module)
- << Filename;
-
- // FIXME: Add support for FixIts in module map files and offer adding the
- // required use declaration.
- if (RequestingModule && getLangOpts().ModulesDeclUse &&
- violatesUseDeclarations(RequestingModule, RequestedModule.getModule()))
- Diag(FilenameLoc, diag::error_undeclared_use_of_module)
- << RequestingModule->getFullModuleName() << Filename;
-}
-
const FileEntry *Preprocessor::LookupFile(
SourceLocation FilenameLoc,
StringRef Filename,
@@ -653,7 +586,8 @@ const FileEntry *Preprocessor::LookupFil
SearchPath, RelativePath, SuggestedModule, SkipCache);
if (FE) {
if (SuggestedModule)
- verifyModuleInclude(FilenameLoc, Filename, FE);
+ HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
+ getModuleForLocation(FilenameLoc), FilenameLoc, Filename, FE);
return FE;
}
Modified: cfe/trunk/test/Modules/Inputs/declare-use/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/declare-use/module.map?rev=197805&r1=197804&r2=197805&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/declare-use/module.map (original)
+++ cfe/trunk/test/Modules/Inputs/declare-use/module.map Fri Dec 20 06:09:36 2013
@@ -20,12 +20,14 @@ module XD {
module XE {
header "e.h"
+ header "unavailable.h"
use XA
use XB
}
module XF {
header "f.h"
+ header "unavailable.h"
use XA
use XB
}
More information about the cfe-commits
mailing list