r223095 - [modules] Track how 'header' directives were written in module map files,

Richard Smith richard-llvm at metafoo.co.uk
Mon Dec 1 16:08:08 PST 2014


Author: rsmith
Date: Mon Dec  1 18:08:08 2014
New Revision: 223095

URL: http://llvm.org/viewvc/llvm-project?rev=223095&view=rev
Log:
[modules] Track how 'header' directives were written in module map files,
rather than trying to extract this information from the FileEntry after the
fact.

This has a number of beneficial effects. For instance, diagnostic messages for
failed module builds give a path relative to the "module root" rather than an
absolute file path, and the contents of the module includes file is no longer
dependent on what files the including TU happened to inspect prior to
triggering the module build.

Modified:
    cfe/trunk/include/clang/Basic/Module.h
    cfe/trunk/include/clang/Lex/ModuleMap.h
    cfe/trunk/include/clang/Lex/Preprocessor.h
    cfe/trunk/lib/Basic/Module.cpp
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/lib/Frontend/FrontendActions.cpp
    cfe/trunk/lib/Lex/HeaderSearch.cpp
    cfe/trunk/lib/Lex/ModuleMap.cpp
    cfe/trunk/lib/Lex/PPDirectives.cpp
    cfe/trunk/lib/Lex/Preprocessor.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/test/Modules/malformed.cpp

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Mon Dec  1 18:08:08 2014
@@ -55,7 +55,12 @@ public:
   /// \brief The parent of this module. This will be NULL for the top-level
   /// module.
   Module *Parent;
-  
+
+  /// \brief The build directory of this module. This is the directory in
+  /// which the module is notionally built, and relative to which its headers
+  /// are found.
+  const DirectoryEntry *Directory;
+
   /// \brief The umbrella header or directory.
   llvm::PointerUnion<const DirectoryEntry *, const FileEntry *> Umbrella;
   
@@ -81,26 +86,28 @@ private:
   mutable llvm::DenseSet<const Module*> VisibleModulesCache;
 
 public:
-  /// \brief The headers that are part of this module.
-  SmallVector<const FileEntry *, 2> NormalHeaders;
-
-  /// \brief The headers that are logically part of this module but
-  /// must be textually included.
-  SmallVector<const FileEntry *, 2> TextualHeaders;
-
-  /// \brief The headers that are private to this module.
-  SmallVector<const FileEntry *, 2> PrivateHeaders;
-
-  /// \brief The headers that are private to this module and are to be
-  /// included textually.
-  SmallVector<const FileEntry *, 2> PrivateTextualHeaders;
-
-  /// \brief The headers that are explicitly excluded from this module.
-  SmallVector<const FileEntry *, 2> ExcludedHeaders;
+  enum HeaderKind {
+    HK_Normal,
+    HK_Textual,
+    HK_Private,
+    HK_PrivateTextual,
+    HK_Excluded
+  };
+  static const int NumHeaderKinds = HK_Excluded + 1;
 
   /// \brief Information about a header directive as found in the module map
   /// file.
-  struct HeaderDirective {
+  struct Header {
+    std::string NameAsWritten;
+    const FileEntry *Entry;
+  };
+
+  /// \brief The headers that are part of this module.
+  SmallVector<Header, 2> Headers[5];
+
+  /// \brief Stored information about a header directive that was found in the
+  /// module map file but has not been resolved to a file.
+  struct UnresolvedHeaderDirective {
     SourceLocation FileNameLoc;
     std::string FileName;
     bool IsUmbrella;
@@ -108,7 +115,7 @@ public:
 
   /// \brief Headers that are mentioned in the module map file but could not be
   /// found on the file system.
-  SmallVector<HeaderDirective, 1> MissingHeaders;
+  SmallVector<UnresolvedHeaderDirective, 1> MissingHeaders;
 
   /// \brief An individual requirement: a feature name and a flag indicating
   /// the required state of that feature.
@@ -303,7 +310,7 @@ public:
   bool isAvailable(const LangOptions &LangOpts, 
                    const TargetInfo &Target,
                    Requirement &Req,
-                   HeaderDirective &MissingHeader) const;
+                   UnresolvedHeaderDirective &MissingHeader) const;
 
   /// \brief Determine whether this module is a submodule.
   bool isSubModule() const { return Parent != nullptr; }

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Mon Dec  1 18:08:08 2014
@@ -440,11 +440,11 @@ public:
 
   /// \brief Adds this header to the given module.
   /// \param Role The role of the header wrt the module.
-  void addHeader(Module *Mod, const FileEntry *Header,
+  void addHeader(Module *Mod, Module::Header Header,
                  ModuleHeaderRole Role);
 
   /// \brief Marks this header as being excluded from the given module.
-  void excludeHeader(Module *Mod, const FileEntry *Header);
+  void excludeHeader(Module *Mod, Module::Header Header);
 
   /// \brief Parse the given module map file, and record any modules we 
   /// encounter.

Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Mon Dec  1 18:08:08 2014
@@ -257,6 +257,11 @@ class Preprocessor : public RefCountedBa
   /// \brief True if we hit the code-completion point.
   bool CodeCompletionReached;
 
+  /// \brief The directory that the main file should be considered to occupy,
+  /// if it does not correspond to a real file (as happens when building a
+  /// module).
+  const DirectoryEntry *MainFileDir;
+
   /// \brief The number of bytes that we will initially skip when entering the
   /// main file, along with a flag that indicates whether skipping this number
   /// of bytes will place the lexer at the start of a line.
@@ -1014,6 +1019,12 @@ public:
     PragmaARCCFCodeAuditedLoc = Loc;
   }
 
+  /// \brief Set the directory in which the main file should be considered
+  /// to have been found, if it is not a real file.
+  void setMainFileDir(const DirectoryEntry *Dir) {
+    MainFileDir = Dir;
+  }
+
   /// \brief Instruct the preprocessor to skip part of the main source file.
   ///
   /// \param Bytes The number of bytes in the preamble to skip.

Modified: cfe/trunk/lib/Basic/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Module.cpp (original)
+++ cfe/trunk/lib/Basic/Module.cpp Mon Dec  1 18:08:08 2014
@@ -26,7 +26,7 @@ using namespace clang;
 
 Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
                bool IsFramework, bool IsExplicit)
-    : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent),
+    : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent), Directory(),
       Umbrella(), ASTFile(nullptr), IsMissingRequirement(false),
       IsAvailable(true), IsFromModuleFile(false), IsFramework(IsFramework),
       IsExplicit(IsExplicit), IsSystem(false), IsExternC(false),
@@ -70,9 +70,9 @@ static bool hasFeature(StringRef Feature
            .Default(Target.hasFeature(Feature));
 }
 
-bool
-Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target,
-                    Requirement &Req, HeaderDirective &MissingHeader) const {
+bool Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target,
+                         Requirement &Req,
+                         UnresolvedHeaderDirective &MissingHeader) const {
   if (IsAvailable)
     return true;
 
@@ -338,19 +338,20 @@ void Module::print(raw_ostream &OS, unsi
     OS << "\n";
   }
 
-  struct HeaderKind {
+  struct {
     StringRef Prefix;
-    const SmallVectorImpl<const FileEntry *> &Headers;
-  } Kinds[] = {{"", NormalHeaders},
-               {"exclude ", ExcludedHeaders},
-               {"textual ", TextualHeaders},
-               {"private ", PrivateHeaders}};
+    HeaderKind Kind;
+  } Kinds[] = {{"", HK_Normal},
+               {"textual ", HK_Textual},
+               {"private ", HK_Private},
+               {"private textual ", HK_PrivateTextual},
+               {"exclude ", HK_Excluded}};
 
   for (auto &K : Kinds) {
-    for (auto *H : K.Headers) {
+    for (auto &H : Headers[K.Kind]) {
       OS.indent(Indent + 2);
       OS << K.Prefix << "header \"";
-      OS.write_escaped(H->getName());
+      OS.write_escaped(H.NameAsWritten);
       OS << "\"\n";
     }
   }

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Dec  1 18:08:08 2014
@@ -1552,7 +1552,7 @@ CompilerInstance::loadModule(SourceLocat
 
     // Check whether this module is available.
     clang::Module::Requirement Requirement;
-    clang::Module::HeaderDirective MissingHeader;
+    clang::Module::UnresolvedHeaderDirective MissingHeader;
     if (!Module->isAvailable(getLangOpts(), getTarget(), Requirement,
                              MissingHeader)) {
       if (MissingHeader.FileNameLoc.isValid()) {
@@ -1581,9 +1581,16 @@ CompilerInstance::loadModule(SourceLocat
                      Module, ImportLoc);
   }
 
+  // Determine whether we're in the #include buffer for a module. The #includes
+  // in that buffer do not qualify as module imports; they're just an
+  // implementation detail of us building the module.
+  bool IsInModuleIncludes = !getLangOpts().CurrentModule.empty() &&
+                            getSourceManager().getFileID(ImportLoc) ==
+                                getSourceManager().getMainFileID();
+
   // If this module import was due to an inclusion directive, create an 
   // implicit import declaration to capture it in the AST.
-  if (IsInclusionDirective && hasASTContext()) {
+  if (IsInclusionDirective && hasASTContext() && !IsInModuleIncludes) {
     TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl();
     ImportDecl *ImportD = ImportDecl::CreateImplicit(getASTContext(), TU,
                                                      ImportLoc, Module,

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Mon Dec  1 18:08:08 2014
@@ -142,17 +142,9 @@ static std::error_code addHeaderInclude(
     Includes += "#import \"";
   else
     Includes += "#include \"";
-  // Use an absolute path for the include; there's no reason to think that
-  // a relative path will work (. might not be on our include path) or that
-  // it will find the same file.
-  if (llvm::sys::path::is_absolute(HeaderName)) {
-    Includes += HeaderName;
-  } else {
-    SmallString<256> Header = HeaderName;
-    if (std::error_code Err = llvm::sys::fs::make_absolute(Header))
-      return Err;
-    Includes += Header;
-  }
+
+  Includes += HeaderName;
+
   Includes += "\"\n";
   if (IsExternC && LangOpts.CPlusPlus)
     Includes += "}\n";
@@ -163,7 +155,16 @@ static std::error_code addHeaderInclude(
                                         SmallVectorImpl<char> &Includes,
                                         const LangOptions &LangOpts,
                                         bool IsExternC) {
-  return addHeaderInclude(Header->getName(), Includes, LangOpts, IsExternC);
+  // Use an absolute path if we don't have a filename as written in the module
+  // map file; this ensures that we will identify the right file independent of
+  // header search paths.
+  if (llvm::sys::path::is_absolute(Header->getName()))
+    return addHeaderInclude(Header->getName(), Includes, LangOpts, IsExternC);
+
+  SmallString<256> AbsName(Header->getName());
+  if (std::error_code Err = llvm::sys::fs::make_absolute(AbsName))
+    return Err;
+  return addHeaderInclude(AbsName, Includes, LangOpts, IsExternC);
 }
 
 /// \brief Collect the set of header includes needed to construct the given 
@@ -182,16 +183,20 @@ collectModuleHeaderIncludes(const LangOp
     return std::error_code();
 
   // Add includes for each of these headers.
-  for (unsigned I = 0, N = Module->NormalHeaders.size(); I != N; ++I) {
-    const FileEntry *Header = Module->NormalHeaders[I];
-    Module->addTopHeader(Header);
-    if (std::error_code Err =
-            addHeaderInclude(Header, Includes, LangOpts, Module->IsExternC))
+  for (Module::Header &H : Module->Headers[Module::HK_Normal]) {
+    Module->addTopHeader(H.Entry);
+    // Use the path as specified in the module map file. We'll look for this
+    // file relative to the module build directory (the directory containing
+    // the module map file) so this will find the same file that we found
+    // while parsing the module map.
+    if (std::error_code Err = addHeaderInclude(H.NameAsWritten, Includes,
+                                               LangOpts, Module->IsExternC))
       return Err;
   }
   // Note that Module->PrivateHeaders will not be a TopHeader.
 
   if (const FileEntry *UmbrellaHeader = Module->getUmbrellaHeader()) {
+    // FIXME: Track the name as written here.
     Module->addTopHeader(UmbrellaHeader);
     if (Module->Parent) {
       // Include the umbrella header for submodules.
@@ -213,25 +218,30 @@ collectModuleHeaderIncludes(const LangOp
           .Cases(".h", ".H", ".hh", ".hpp", true)
           .Default(false))
         continue;
-      
+
+      const FileEntry *Header = FileMgr.getFile(Dir->path());
+      // FIXME: This shouldn't happen unless there is a file system race. Is
+      // that worth diagnosing?
+      if (!Header)
+        continue;
+
       // If this header is marked 'unavailable' in this module, don't include 
       // it.
-      if (const FileEntry *Header = FileMgr.getFile(Dir->path())) {
-        if (ModMap.isHeaderUnavailableInModule(Header, Module))
-          continue;
-        Module->addTopHeader(Header);
-      }
-      
+      if (ModMap.isHeaderUnavailableInModule(Header, Module))
+        continue;
+
       // Include this header as part of the umbrella directory.
-      if (std::error_code Err = addHeaderInclude(Dir->path(), Includes,
-                                                 LangOpts, Module->IsExternC))
+      // FIXME: Track the name as written through to here.
+      Module->addTopHeader(Header);
+      if (std::error_code Err =
+              addHeaderInclude(Header, Includes, LangOpts, Module->IsExternC))
         return Err;
     }
 
     if (EC)
       return EC;
   }
-  
+
   // Recurse into submodules.
   for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
                                       SubEnd = Module->submodule_end();
@@ -288,7 +298,7 @@ bool GenerateModuleAction::BeginSourceFi
 
   // Check whether we can build this module at all.
   clang::Module::Requirement Requirement;
-  clang::Module::HeaderDirective MissingHeader;
+  clang::Module::UnresolvedHeaderDirective MissingHeader;
   if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement,
                            MissingHeader)) {
     if (MissingHeader.FileNameLoc.isValid()) {
@@ -317,6 +327,7 @@ bool GenerateModuleAction::BeginSourceFi
   SmallString<256> HeaderContents;
   std::error_code Err = std::error_code();
   if (const FileEntry *UmbrellaHeader = Module->getUmbrellaHeader())
+    // FIXME: Track the file name as written.
     Err = addHeaderInclude(UmbrellaHeader, HeaderContents, CI.getLangOpts(),
                            Module->IsExternC);
   if (!Err)
@@ -331,6 +342,10 @@ bool GenerateModuleAction::BeginSourceFi
     return false;
   }
 
+  // Inform the preprocessor that includes from within the input buffer should
+  // be resolved relative to the build directory of the module map file.
+  CI.getPreprocessor().setMainFileDir(Module->Directory);
+
   std::unique_ptr<llvm::MemoryBuffer> InputBuffer =
       llvm::MemoryBuffer::getMemBufferCopy(HeaderContents,
                                            Module::getModuleInputBufferName());

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Mon Dec  1 18:08:08 2014
@@ -624,11 +624,20 @@ const FileEntry *HeaderSearch::LookupFil
       // FIXME: We don't cache the result of getFileInfo across the call to
       // getFileAndSuggestModule, because it's a reference to an element of
       // a container that could be reallocated across this call.
+      //
+      // FIXME: If we have no includer, that means we're processing a #include
+      // from a module build. We should treat this as a system header if we're
+      // building a [system] module.
       bool IncluderIsSystemHeader =
-          getFileInfo(Includer).DirInfo != SrcMgr::C_User;
+          Includer && getFileInfo(Includer).DirInfo != SrcMgr::C_User;
       if (const FileEntry *FE = getFileAndSuggestModule(
               *this, TmpDir.str(), IncluderAndDir.second,
               IncluderIsSystemHeader, SuggestedModule)) {
+        if (!Includer) {
+          assert(First && "only first includer can have no file");
+          return FE;
+        }
+
         // Leave CurDir unset.
         // This file is a system header or C++ unfriendly if the old file is.
         //
@@ -770,7 +779,7 @@ const FileEntry *HeaderSearch::LookupFil
   // a header in a framework that is currently being built, and we couldn't
   // resolve "foo.h" any other way, change the include to <Foo/foo.h>, where
   // "Foo" is the name of the framework in which the including header was found.
-  if (!Includers.empty() && !isAngled &&
+  if (!Includers.empty() && Includers.front().first && !isAngled &&
       Filename.find('/') == StringRef::npos) {
     HeaderFileInfo &IncludingHFI = getFileInfo(Includers.front().first);
     if (IncludingHFI.IndexHeaderMapHeader) {
@@ -1079,7 +1088,9 @@ bool HeaderSearch::hasModuleMap(StringRe
       return false;
 
     // Try to load the module map file in this directory.
-    switch (loadModuleMapFile(Dir, IsSystem, /*IsFramework*/false)) {
+    switch (loadModuleMapFile(Dir, IsSystem,
+                              llvm::sys::path::extension(Dir->getName()) ==
+                                  ".framework")) {
     case LMM_NewlyLoaded:
     case LMM_AlreadyLoaded:
       // Success. All of the directories we stepped through inherit this module

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Mon Dec  1 18:08:08 2014
@@ -220,12 +220,14 @@ static bool violatesPrivateInclude(Modul
   // as obtained from the lookup and as obtained from the module.
   // This check is not cheap, so enable it only for debugging.
   bool IsPrivate = false;
-  SmallVectorImpl<const FileEntry *> *HeaderList[] =
-      {&RequestedModule->PrivateHeaders,
-       &RequestedModule->PrivateTextualHeaders};
+  SmallVectorImpl<Module::Header> *HeaderList[] =
+      {&RequestedModule->Headers[Module::HK_Private],
+       &RequestedModule->Headers[Module::HK_PrivateTextual]};
   for (auto *Hdrs : HeaderList)
     IsPrivate |=
-        std::find(Hdrs->begin(), Hdrs->end(), IncFileEnt) != Hdrs->end();
+        std::find_if(Hdrs->begin(), Hdrs->end(), [&](const Module::Header &H) {
+          return H.Entry == IncFileEnt;
+        }) != Hdrs->end();
   assert(IsPrivate == IsPrivateRole && "inconsistent headers and roles");
 #endif
   return IsPrivateRole &&
@@ -778,40 +780,40 @@ void ModuleMap::setUmbrellaDir(Module *M
   UmbrellaDirs[UmbrellaDir] = Mod;
 }
 
-void ModuleMap::addHeader(Module *Mod, const FileEntry *Header,
-                          ModuleHeaderRole Role) {
+static Module::HeaderKind headerRoleToKind(ModuleMap::ModuleHeaderRole Role) {
   switch ((int)Role) {
-  default:
-    llvm_unreachable("unknown header role");
-  case NormalHeader:
-    Mod->NormalHeaders.push_back(Header);
-    break;
-  case PrivateHeader:
-    Mod->PrivateHeaders.push_back(Header);
-    break;
-  case TextualHeader:
-    Mod->TextualHeaders.push_back(Header);
-    break;
-  case PrivateHeader | TextualHeader:
-    Mod->PrivateTextualHeaders.push_back(Header);
-    break;
+  default: llvm_unreachable("unknown header role");
+  case ModuleMap::NormalHeader:
+    return Module::HK_Normal;
+  case ModuleMap::PrivateHeader:
+    return Module::HK_Private;
+  case ModuleMap::TextualHeader:
+    return Module::HK_Textual;
+  case ModuleMap::PrivateHeader | ModuleMap::TextualHeader:
+    return Module::HK_PrivateTextual;
   }
+}
 
+void ModuleMap::addHeader(Module *Mod, Module::Header Header,
+                          ModuleHeaderRole Role) {
   if (!(Role & TextualHeader)) {
     bool isCompilingModuleHeader = Mod->getTopLevelModule() == CompilingModule;
-    HeaderInfo.MarkFileModuleHeader(Header, Role, isCompilingModuleHeader);
+    HeaderInfo.MarkFileModuleHeader(Header.Entry, Role,
+                                    isCompilingModuleHeader);
   }
-  Headers[Header].push_back(KnownHeader(Mod, Role));
-}
+  Headers[Header.Entry].push_back(KnownHeader(Mod, Role));
 
-void ModuleMap::excludeHeader(Module *Mod, const FileEntry *Header) {
-  Mod->ExcludedHeaders.push_back(Header);
+  Mod->Headers[headerRoleToKind(Role)].push_back(std::move(Header));
+}
 
+void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) {
   // Add this as a known header so we won't implicitly add it to any
   // umbrella directory module.
   // FIXME: Should we only exclude it from umbrella modules within the
   // specified module?
-  (void) Headers[Header];
+  (void) Headers[Header.Entry];
+
+  Mod->Headers[Module::HK_Excluded].push_back(std::move(Header));
 }
 
 const FileEntry *
@@ -1447,6 +1449,7 @@ void ModuleMapParser::parseModuleDecl()
     ActiveModule->IsSystem = true;
   if (Attrs.IsExternC)
     ActiveModule->IsExternC = true;
+  ActiveModule->Directory = Directory;
 
   bool Done = false;
   do {
@@ -1699,7 +1702,7 @@ void ModuleMapParser::parseHeaderDecl(MM
     HadError = true;
     return;
   }
-  Module::HeaderDirective Header;
+  Module::UnresolvedHeaderDirective Header;
   Header.FileName = Tok.getString();
   Header.FileNameLoc = consumeToken();
   
@@ -1714,33 +1717,39 @@ void ModuleMapParser::parseHeaderDecl(MM
   // Look for this file.
   const FileEntry *File = nullptr;
   const FileEntry *BuiltinFile = nullptr;
-  SmallString<128> PathName;
+  SmallString<128> RelativePathName;
   if (llvm::sys::path::is_absolute(Header.FileName)) {
-    PathName = Header.FileName;
-    File = SourceMgr.getFileManager().getFile(PathName);
+    RelativePathName = Header.FileName;
+    File = SourceMgr.getFileManager().getFile(RelativePathName);
   } else {
     // Search for the header file within the search directory.
-    PathName = Directory->getName();
-    unsigned PathLength = PathName.size();
+    SmallString<128> FullPathName(Directory->getName());
+    unsigned FullPathLength = FullPathName.size();
     
     if (ActiveModule->isPartOfFramework()) {
-      appendSubframeworkPaths(ActiveModule, PathName);
+      appendSubframeworkPaths(ActiveModule, RelativePathName);
       
       // Check whether this file is in the public headers.
-      llvm::sys::path::append(PathName, "Headers", Header.FileName);
-      File = SourceMgr.getFileManager().getFile(PathName);
+      llvm::sys::path::append(RelativePathName, "Headers", Header.FileName);
+      llvm::sys::path::append(FullPathName, RelativePathName.str());
+      File = SourceMgr.getFileManager().getFile(FullPathName);
       
       if (!File) {
         // Check whether this file is in the private headers.
-        PathName.resize(PathLength);
-        llvm::sys::path::append(PathName, "PrivateHeaders", Header.FileName);
-        File = SourceMgr.getFileManager().getFile(PathName);
+        // FIXME: Should we retain the subframework paths here?
+        RelativePathName.clear();
+        FullPathName.resize(FullPathLength);
+        llvm::sys::path::append(RelativePathName, "PrivateHeaders",
+                                Header.FileName);
+        llvm::sys::path::append(FullPathName, RelativePathName.str());
+        File = SourceMgr.getFileManager().getFile(FullPathName);
       }
     } else {
       // Lookup for normal headers.
-      llvm::sys::path::append(PathName, Header.FileName);
-      File = SourceMgr.getFileManager().getFile(PathName);
-      
+      llvm::sys::path::append(RelativePathName, Header.FileName);
+      llvm::sys::path::append(FullPathName, RelativePathName.str());
+      File = SourceMgr.getFileManager().getFile(FullPathName);
+
       // If this is a system module with a top-level header, this header
       // may have a counterpart (or replacement) in the set of headers
       // supplied by Clang. Find that builtin header.
@@ -1750,18 +1759,19 @@ void ModuleMapParser::parseHeaderDecl(MM
         SmallString<128> BuiltinPathName(BuiltinIncludeDir->getName());
         llvm::sys::path::append(BuiltinPathName, Header.FileName);
         BuiltinFile = SourceMgr.getFileManager().getFile(BuiltinPathName);
-        
+
         // If Clang supplies this header but the underlying system does not,
         // just silently swap in our builtin version. Otherwise, we'll end
         // up adding both (later).
         if (!File && BuiltinFile) {
           File = BuiltinFile;
+          RelativePathName = BuiltinPathName;
           BuiltinFile = nullptr;
         }
       }
     }
   }
-  
+
   // FIXME: We shouldn't be eagerly stat'ing every file named in a module map.
   // Come up with a lazy way to do this.
   if (File) {
@@ -1776,16 +1786,23 @@ void ModuleMapParser::parseHeaderDecl(MM
         Map.setUmbrellaHeader(ActiveModule, File);
       }
     } else if (LeadingToken == MMToken::ExcludeKeyword) {
-      Map.excludeHeader(ActiveModule, File);
+      Map.excludeHeader(ActiveModule,
+                        Module::Header{RelativePathName.str(), File});
     } else {
       // If there is a builtin counterpart to this file, add it now, before
       // the "real" header, so we build the built-in one first when building
       // the module.
       if (BuiltinFile)
-        Map.addHeader(ActiveModule, BuiltinFile, Role);
+        // FIXME: Taking the name from the FileEntry is unstable and can give
+        // different results depending on how we've previously named that file
+        // in this build.
+        Map.addHeader(ActiveModule,
+                      Module::Header{BuiltinFile->getName(), BuiltinFile},
+                      Role);
 
       // Record this header.
-      Map.addHeader(ActiveModule, File, Role);
+      Map.addHeader(ActiveModule, Module::Header{RelativePathName.str(), File},
+                    Role);
     }
   } else if (LeadingToken != MMToken::ExcludeKeyword) {
     // Ignore excluded header files. They're optional anyway.

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Dec  1 18:08:08 2014
@@ -550,14 +550,22 @@ const FileEntry *Preprocessor::LookupFil
     const FileEntry *FileEnt = SourceMgr.getFileEntryForID(FID);
 
     // If there is no file entry associated with this file, it must be the
-    // predefines buffer.  Any other file is not lexed with a normal lexer, so
-    // it won't be scanned for preprocessor directives.   If we have the
-    // predefines buffer, resolve #include references (which come from the
-    // -include command line argument) from the current working directory
-    // instead of relative to the main file.
+    // predefines buffer or the module includes buffer. Any other file is not
+    // lexed with a normal lexer, so it won't be scanned for preprocessor
+    // directives.
+    //
+    // If we have the predefines buffer, resolve #include references (which come
+    // from the -include command line argument) from the current working
+    // directory instead of relative to the main file.
+    //
+    // If we have the module includes buffer, resolve #include references (which
+    // come from header declarations in the module map) relative to the module
+    // map file.
     if (!FileEnt) {
-      FileEnt = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
-      if (FileEnt)
+      if (FID == SourceMgr.getMainFileID() && MainFileDir)
+        Includers.push_back(std::make_pair(nullptr, MainFileDir));
+      else if ((FileEnt =
+                    SourceMgr.getFileEntryForID(SourceMgr.getMainFileID())))
         Includers.push_back(std::make_pair(FileEnt, FileMgr.getDirectory(".")));
     } else {
       Includers.push_back(std::make_pair(FileEnt, FileEnt->getDir()));

Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
+++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Dec  1 18:08:08 2014
@@ -71,7 +71,7 @@ Preprocessor::Preprocessor(IntrusiveRefC
       CodeComplete(nullptr), CodeCompletionFile(nullptr),
       CodeCompletionOffset(0), LastTokenWasAt(false),
       ModuleImportExpectsIdentifier(false), CodeCompletionReached(0),
-      SkipMainFilePreamble(0, true), CurPPLexer(nullptr),
+      MainFileDir(nullptr), SkipMainFilePreamble(0, true), CurPPLexer(nullptr),
       CurDirLookup(nullptr), CurLexerKind(CLK_Lexer), CurSubmodule(nullptr),
       Callbacks(nullptr), MacroArgCache(nullptr), Record(nullptr),
       MIChainHead(nullptr), DeserialMIChainHead(nullptr) {

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Dec  1 18:08:08 2014
@@ -1559,7 +1559,13 @@ HeaderFileInfoTrait::ReadData(internal_k
       FileManager &FileMgr = Reader.getFileManager();
       ModuleMap &ModMap =
           Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
-      ModMap.addHeader(Mod, FileMgr.getFile(key.Filename), HFI.getHeaderRole());
+      // FIXME: This is wrong. We should track the filename as written; this
+      // information should be propagated through the SUBMODULE_HEADER etc
+      // records rather than from here.
+      // FIXME: We don't ever mark excluded headers.
+      ModMap.addHeader(
+          Mod, Module::Header{key.Filename, FileMgr.getFile(key.Filename)},
+          HFI.getHeaderRole());
     }
   }
 

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Mon Dec  1 18:08:08 2014
@@ -2511,24 +2511,31 @@ void ASTWriter::WriteSubmodules(Module *
 
     // Emit the headers.
     struct {
-      unsigned Kind;
+      unsigned RecordKind;
       unsigned Abbrev;
-      ArrayRef<const FileEntry*> Headers;
+      Module::HeaderKind HeaderKind;
     } HeaderLists[] = {
-      {SUBMODULE_HEADER, HeaderAbbrev, Mod->NormalHeaders},
-      {SUBMODULE_TEXTUAL_HEADER, TextualHeaderAbbrev, Mod->TextualHeaders},
-      {SUBMODULE_PRIVATE_HEADER, PrivateHeaderAbbrev, Mod->PrivateHeaders},
+      {SUBMODULE_HEADER, HeaderAbbrev, Module::HK_Normal},
+      {SUBMODULE_TEXTUAL_HEADER, TextualHeaderAbbrev, Module::HK_Textual},
+      {SUBMODULE_PRIVATE_HEADER, PrivateHeaderAbbrev, Module::HK_Private},
       {SUBMODULE_PRIVATE_TEXTUAL_HEADER, PrivateTextualHeaderAbbrev,
-       Mod->PrivateTextualHeaders},
-      {SUBMODULE_EXCLUDED_HEADER, ExcludedHeaderAbbrev, Mod->ExcludedHeaders},
-      {SUBMODULE_TOPHEADER, TopHeaderAbbrev,
-       Mod->getTopHeaders(PP->getFileManager())}
+        Module::HK_PrivateTextual},
+      {SUBMODULE_EXCLUDED_HEADER, ExcludedHeaderAbbrev, Module::HK_Excluded}
     };
     for (auto &HL : HeaderLists) {
       Record.clear();
-      Record.push_back(HL.Kind);
-      for (auto *H : HL.Headers)
-        Stream.EmitRecordWithBlob(HL.Abbrev, Record, H->getName());
+      Record.push_back(HL.RecordKind);
+      for (auto &H : Mod->Headers[HL.HeaderKind])
+        Stream.EmitRecordWithBlob(HL.Abbrev, Record, H.NameAsWritten);
+    }
+
+    // Emit the top headers.
+    {
+      auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
+      Record.clear();
+      Record.push_back(SUBMODULE_TOPHEADER);
+      for (auto *H : TopHeaders)
+        Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, H->getName());
     }
 
     // Emit the imports. 

Modified: cfe/trunk/test/Modules/malformed.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/malformed.cpp?rev=223095&r1=223094&r2=223095&view=diff
==============================================================================
--- cfe/trunk/test/Modules/malformed.cpp (original)
+++ cfe/trunk/test/Modules/malformed.cpp Mon Dec  1 18:08:08 2014
@@ -1,23 +1,27 @@
+// This test explicitly cd's to the test/Modules directory so that we can test
+// that filenames found via relative -I paths are printed correctly.
+//
 // RUN: rm -rf %t
-// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs/malformed -DHEADER="a1.h" %s 2>&1 | FileCheck %s --check-prefix=CHECK-A
-// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs/malformed -DHEADER="b1.h" %s 2>&1 | FileCheck %s --check-prefix=CHECK-B
+// RUN: cd %S
+// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -I Inputs/malformed -DHEADER="a1.h" %s 2>&1 | FileCheck %s --check-prefix=CHECK-A
+// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -I Inputs/malformed -DHEADER="b1.h" %s 2>&1 | FileCheck %s --check-prefix=CHECK-B
 
 #define STR2(x) #x
 #define STR(x) STR2(x)
 #include STR(HEADER)
 
 // CHECK-A: While building module 'malformed_a'
-// CHECK-A: a1.h:1:{{.*}} error: expected '}'
-// CHECK-A: a1.h:1:{{.*}} note: to match this '{'
+// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} error: expected '}'
+// CHECK-A: {{^}}Inputs/malformed/a1.h:1:{{.*}} note: to match this '{'
 //
 // CHECK-A: While building module 'malformed_a'
-// CHECK-A: a2.h:1:{{.*}} error: extraneous closing brace
+// CHECK-A: {{^}}Inputs/malformed/a2.h:1:{{.*}} error: extraneous closing brace
 
 // CHECK-B: While building module 'malformed_b'
-// CHECK-B: b1.h:2:{{.*}} error: expected '}'
-// CHECK-B: b1.h:1:{{.*}} note: to match this '{'
-// CHECK-B: b1.h:3:{{.*}} error: extraneous closing brace ('}')
+// CHECK-B: {{^}}Inputs/malformed/b1.h:2:{{.*}} error: expected '}'
+// CHECK-B: {{^}}Inputs/malformed/b1.h:1:{{.*}} note: to match this '{'
+// CHECK-B: {{^}}Inputs/malformed/b1.h:3:{{.*}} error: extraneous closing brace ('}')
 //
 // CHECK-B: While building module 'malformed_b'
-// CHECK-B: b2.h:1:{{.*}} error: redefinition of 'g'
-// CHECK-B: b2.h:1:{{.*}} note: previous definition is here
+// CHECK-B: {{^}}Inputs/malformed/b2.h:1:{{.*}} error: redefinition of 'g'
+// CHECK-B: {{^}}Inputs/malformed/b2.h:1:{{.*}} note: previous definition is here





More information about the cfe-commits mailing list