From: Richard Smith Date: Wed, 5 Mar 2014 20:51:45 +0000 (+0000) Subject: If a #include finds a file relative to the current file, don't forget to check X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a889b8ad628ca8640834812b2e144ee8c908a788;p=clang If a #include finds a file relative to the current file, don't forget to check whether it's part of a module. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@203005 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp index fa0b3f7596..11af5854a2 100644 --- a/lib/Lex/HeaderSearch.cpp +++ b/lib/Lex/HeaderSearch.cpp @@ -219,6 +219,32 @@ const char *DirectoryLookup::getName() const { return getHeaderMap()->getFileName(); } +static const FileEntry * +getFileAndSuggestModule(HeaderSearch &HS, StringRef FileName, + const DirectoryEntry *Dir, bool IsSystemHeaderDir, + ModuleMap::KnownHeader *SuggestedModule) { + // If we have a module map that might map this header, load it and + // check whether we'll have a suggestion for a module. + HS.hasModuleMap(FileName, Dir, IsSystemHeaderDir); + if (SuggestedModule) { + const FileEntry *File = HS.getFileMgr().getFile(FileName, + /*OpenFile=*/false); + if (File) { + // If there is a module that corresponds to this header, suggest it. + *SuggestedModule = HS.findModuleForHeader(File); + + // FIXME: This appears to be a no-op. We loaded the module map for this + // directory at the start of this function. + if (!SuggestedModule->getModule() && + HS.hasModuleMap(FileName, Dir, IsSystemHeaderDir)) + *SuggestedModule = HS.findModuleForHeader(File); + } + + return File; + } + + return HS.getFileMgr().getFile(FileName, /*openFile=*/true); +} /// LookupFile - Lookup the specified file in this search path, returning it /// if it exists or returning null if not. @@ -246,25 +272,10 @@ const FileEntry *DirectoryLookup::LookupFile( RelativePath->clear(); RelativePath->append(Filename.begin(), Filename.end()); } - - // If we have a module map that might map this header, load it and - // check whether we'll have a suggestion for a module. - HS.hasModuleMap(TmpDir, getDir(), isSystemHeaderDirectory()); - if (SuggestedModule) { - const FileEntry *File = HS.getFileMgr().getFile(TmpDir.str(), - /*openFile=*/false); - if (!File) - return File; - - // If there is a module that corresponds to this header, suggest it. - *SuggestedModule = HS.findModuleForHeader(File); - if (!SuggestedModule->getModule() && - HS.hasModuleMap(TmpDir, getDir(), isSystemHeaderDirectory())) - *SuggestedModule = HS.findModuleForHeader(File); - return File; - } - - return HS.getFileMgr().getFile(TmpDir.str(), /*openFile=*/true); + + return getFileAndSuggestModule(HS, TmpDir.str(), getDir(), + isSystemHeaderDirectory(), + SuggestedModule); } if (isFramework()) @@ -573,6 +584,7 @@ const FileEntry *HeaderSearch::LookupFile( // This is the header that MSVC's header search would have found. const FileEntry *MSFE = 0; + ModuleMap::KnownHeader MSSuggestedModule; // Unless disabled, check to see if the file is in the #includer's // directory. This cannot be based on CurDir, because each includer could be @@ -590,15 +602,18 @@ const FileEntry *HeaderSearch::LookupFile( TmpDir = Includer->getDir()->getName(); TmpDir.push_back('/'); TmpDir.append(Filename.begin(), Filename.end()); + + HeaderFileInfo &FromHFI = getFileInfo(Includer); if (const FileEntry *FE = - FileMgr.getFile(TmpDir.str(), /*openFile=*/true)) { + getFileAndSuggestModule(*this, TmpDir.str(), Includer->getDir(), + FromHFI.DirInfo != SrcMgr::C_User, + SuggestedModule)) { // Leave CurDir unset. // This file is a system header or C++ unfriendly if the old file is. // // Note that we only use one of FromHFI/ToHFI at once, due to potential // reallocation of the underlying vector potentially making the first // reference binding dangling. - HeaderFileInfo &FromHFI = getFileInfo(Includer); unsigned DirInfo = FromHFI.DirInfo; bool IndexHeaderMapHeader = FromHFI.IndexHeaderMapHeader; StringRef Framework = FromHFI.Framework; @@ -629,6 +644,10 @@ const FileEntry *HeaderSearch::LookupFile( return FE; } else { MSFE = FE; + if (SuggestedModule) { + MSSuggestedModule = *SuggestedModule; + *SuggestedModule = ModuleMap::KnownHeader(); + } break; } } @@ -709,8 +728,11 @@ const FileEntry *HeaderSearch::LookupFile( } } - if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc)) + if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc)) { + if (SuggestedModule) + *SuggestedModule = MSSuggestedModule; return MSFE; + } // Remember this location for the next lookup we do. CacheLookup.second = i; @@ -734,19 +756,26 @@ const FileEntry *HeaderSearch::LookupFile( ScratchFilename, IncludeLoc, /*isAngled=*/true, FromDir, CurDir, Includers.front(), SearchPath, RelativePath, SuggestedModule); - if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc)) + if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc)) { + if (SuggestedModule) + *SuggestedModule = MSSuggestedModule; return MSFE; + } std::pair &CacheLookup = LookupFileCache.GetOrCreateValue(Filename).getValue(); CacheLookup.second = LookupFileCache.GetOrCreateValue(ScratchFilename).getValue().second; + // FIXME: SuggestedModule. return FE; } } - if (checkMSVCHeaderSearch(Diags, MSFE, 0, IncludeLoc)) + if (checkMSVCHeaderSearch(Diags, MSFE, 0, IncludeLoc)) { + if (SuggestedModule) + *SuggestedModule = MSSuggestedModule; return MSFE; + } // Otherwise, didn't find it. Remember we didn't find this. CacheLookup.second = SearchDirs.size(); diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp index 0d9cd10953..86ffa5288a 100644 --- a/lib/Lex/ModuleMap.cpp +++ b/lib/Lex/ModuleMap.cpp @@ -297,6 +297,8 @@ ModuleMap::findModuleForHeader(const FileEntry *File, Result = *I; // If 'File' is a public header of this module, this is as good as we // are going to get. + // FIXME: If we have a RequestingModule, we should prefer the header from + // that module. if (I->getRole() == ModuleMap::NormalHeader) break; } diff --git a/test/Modules/Inputs/module.map b/test/Modules/Inputs/module.map index 362a175f60..67b3a5a0eb 100644 --- a/test/Modules/Inputs/module.map +++ b/test/Modules/Inputs/module.map @@ -290,3 +290,9 @@ module recursive_visibility_b { module recursive_visibility_c { header "recursive_visibility_c.h" } +module recursive1 { + header "recursive1.h" +} +module recursive2 { + header "recursive2.h" +} diff --git a/test/Modules/Inputs/recursive1.h b/test/Modules/Inputs/recursive1.h new file mode 100644 index 0000000000..8cb5917397 --- /dev/null +++ b/test/Modules/Inputs/recursive1.h @@ -0,0 +1 @@ +#include "recursive2.h" diff --git a/test/Modules/Inputs/recursive2.h b/test/Modules/Inputs/recursive2.h new file mode 100644 index 0000000000..d9480aa1e1 --- /dev/null +++ b/test/Modules/Inputs/recursive2.h @@ -0,0 +1 @@ +#include "recursive1.h" diff --git a/test/Modules/recursive.c b/test/Modules/recursive.c new file mode 100644 index 0000000000..668ba38dff --- /dev/null +++ b/test/Modules/recursive.c @@ -0,0 +1,11 @@ +// RUN: rm -rf %t +// RUN: not %clang_cc1 -fmodules -x objective-c -fmodules-cache-path=%t -I %S/Inputs %s 2>&1 | FileCheck %s +#include "recursive1.h" + +// FIXME: rm -rf %t +// FIXME: not %clang_cc1 -fmodules -x objective-c -fmodules-cache-path=%t -emit-module -fmodule-name=recursive1 %S/Inputs/module.map 2>&1 | FileCheck %s + +// CHECK: While building module 'recursive1'{{( imported from .*/recursive.c:3)?}}: +// CHECK-NEXT: While building module 'recursive2' imported from {{.*}}Inputs/recursive1.h:1: +// CHECK-NEXT: In file included from :1: +// CHECK-NEXT: recursive2.h:1:10: fatal error: cyclic dependency in module 'recursive1': recursive1 -> recursive2 -> recursive1 diff --git a/test/Modules/submodules.cpp b/test/Modules/submodules.cpp index 12bf87f630..a18138c709 100644 --- a/test/Modules/submodules.cpp +++ b/test/Modules/submodules.cpp @@ -31,9 +31,7 @@ hash_map ints_to_floats2; extern MyTypeA import_self_test_a; // expected-error {{must be imported from module 'import_self.a'}} // expected-note@import-self-a.h:1 {{here}} extern MyTypeC import_self_test_c; -// FIXME: This should be valid; import_self.b re-exports import_self.d. -extern MyTypeD import_self_test_d; // expected-error {{must be imported from module 'import_self.d'}} -// expected-note@import-self-d.h:1 {{here}} +extern MyTypeD import_self_test_d; // expected-error@Inputs/submodules/module.map:15{{header 'missing.h' not found}} @import missing_headers.missing;