]> granicus.if.org Git - clang/commitdiff
Enable layering check in unavailable modules.
authorDaniel Jasper <djasper@google.com>
Fri, 20 Dec 2013 12:09:36 +0000 (12:09 +0000)
committerDaniel Jasper <djasper@google.com>
Fri, 20 Dec 2013 12:09:36 +0000 (12:09 +0000)
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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@197805 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Lex/ModuleMap.h
include/clang/Lex/Preprocessor.h
lib/Lex/ModuleMap.cpp
lib/Lex/PPDirectives.cpp
test/Modules/Inputs/declare-use/module.map

index 3c1aff7cfb54df8d9c678597c42791db4c22db51..1cd6d385b6f01135f7193c368cc60f667bce5f60 100644 (file)
@@ -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'.
index 2491011aa6d9dfbd713e7de8d851e46afd4061f1..7862ed3109c3eb5c0bcc2ab446a1b6f7de3ddee3 100644 (file)
@@ -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);
index b9a4703128956aec4d69dfd1e138faaf73b563a9..05238278bad93ba99fc1e795039187c8a93ea50f 100644 (file)
@@ -159,21 +159,115 @@ static bool isBuiltinHeader(StringRef FileName) {
            .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 FileEntry *File,
       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 FileEntry *File,
       // 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;
index 1b33f45f4daf15552a1f69e5093e571e7e85b969..b93a39188ac6c38e368b8cba39d014dda974689e 100644 (file)
@@ -550,73 +550,6 @@ Module *Preprocessor::getModuleForLocation(SourceLocation FilenameLoc) {
   }
 }
 
-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::LookupFile(
       SearchPath, RelativePath, SuggestedModule, SkipCache);
   if (FE) {
     if (SuggestedModule)
-      verifyModuleInclude(FilenameLoc, Filename, FE);
+      HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
+          getModuleForLocation(FilenameLoc), FilenameLoc, Filename, FE);
     return FE;
   }
 
index ba615b67144ac182826b447b62a3fac23260f806..40f071289a78db20a521ff9644f701256d07dab5 100644 (file)
@@ -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
 }