Hi doug.gregor, rsmith, djasper,
When searching for a header, Clang will first try to look in the current
directory. But if we find the header in this case, we don't set
'SuggestedModule', so the header is included in non-modular way, even if
module.map has an entry for it. Fixing this issue fixes module tests for
-fmodules-decluse, and test/Modules/submodules.cpp.
test/Modules/submodules.cpp already had a FIXME that this patch fixes.
-fmodules-decluse tests had dormant errors that this patch uncovers and fixes.
Specifically, XC and XD #included "b.h", but did not specify the use dependency
in module.map.
Please review.
http://llvm-reviews.chandlerc.com/D2374
Files:
include/clang/Lex/HeaderSearch.h
include/clang/Lex/Preprocessor.h
lib/Frontend/VerifyDiagnosticConsumer.cpp
lib/Lex/HeaderSearch.cpp
lib/Lex/PPDirectives.cpp
test/Modules/Inputs/declare-use/h.h
test/Modules/Inputs/declare-use/module.map
test/Modules/declare-use2.cpp
test/Modules/submodules.cpp
Index: include/clang/Lex/HeaderSearch.h
===================================================================
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -125,6 +125,10 @@
void setHeaderRole(ModuleMap::ModuleHeaderRole Role) {
HeaderRole = Role;
}
+
+ SrcMgr::CharacteristicKind getDirInfo() const {
+ return static_cast<SrcMgr::CharacteristicKind>(DirInfo);
+ }
};
/// \brief An external source of header file information, which may supply
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -1271,18 +1271,37 @@
/// in ""'s.
bool GetIncludeFilenameSpelling(SourceLocation Loc,StringRef &Filename);
+private:
+ const FileEntry *lookupFileImpl(SourceLocation FilenameLoc,
+ StringRef Filename,
+ bool IsAngled,
+ const DirectoryLookup *FromDir,
+ const DirectoryLookup *&CurDir,
+ SmallVectorImpl<char> *SearchPath,
+ SmallVectorImpl<char> *RelativePath,
+ ModuleMap::KnownHeader *SuggestedModule,
+ bool SkipCache);
+
+public:
/// \brief Given a "foo" or \<foo> reference, look up the indicated file.
///
/// Returns null on failure. \p isAngled indicates whether the file
/// reference is for system \#include's or not (i.e. using <> instead of "").
const FileEntry *LookupFile(SourceLocation FilenameLoc, StringRef Filename,
- bool isAngled, const DirectoryLookup *FromDir,
+ bool IsAngled, const DirectoryLookup *FromDir,
const DirectoryLookup *&CurDir,
SmallVectorImpl<char> *SearchPath,
SmallVectorImpl<char> *RelativePath,
ModuleMap::KnownHeader *SuggestedModule,
bool SkipCache = false);
+ /// \brief Lookup a file using normal rules, like \#include, but don't count
+ /// this as a module use. This should be used for diagnostics purposes when
+ /// the file is not actually included in the translation unit.
+ const FileEntry *lookupFileForDiagnostics(SourceLocation FilenameLoc,
+ StringRef Filename,
+ bool IsAngled);
+
/// \brief Get the DirectoryLookup structure used to find the current
/// FileEntry, if CurLexer is non-null and if applicable.
///
Index: lib/Frontend/VerifyDiagnosticConsumer.cpp
===================================================================
--- lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -370,9 +370,8 @@
PH.Advance();
// Lookup file via Preprocessor, like a #include.
- const DirectoryLookup *CurDir;
- const FileEntry *FE = PP->LookupFile(Pos, Filename, false, NULL, CurDir,
- NULL, NULL, 0);
+ const FileEntry *FE =
+ PP->lookupFileForDiagnostics(Pos, Filename, /*IsAngled=*/false);
if (!FE) {
Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin),
diag::err_verify_missing_file) << Filename << KindStr;
Index: lib/Lex/HeaderSearch.cpp
===================================================================
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -549,38 +549,35 @@
// a subsequent include of "baz.h" should resolve to "whatever/foo/baz.h".
// This search is not done for <> headers.
if (CurFileEnt && !isAngled && !NoCurDirSearch) {
- SmallString<1024> TmpDir;
- // Concatenate the requested file onto the directory.
- // FIXME: Portability. Filename concatenation should be in sys::Path.
- TmpDir += CurFileEnt->getDir()->getName();
- TmpDir.push_back('/');
- TmpDir.append(Filename.begin(), Filename.end());
- if (const FileEntry *FE = FileMgr.getFile(TmpDir.str(),/*openFile=*/true)) {
+ HeaderFileInfo &IncludingHFI = getFileInfo(CurFileEnt);
+ bool InUserSpecifiedSystemFramework = false;
+ DirectoryLookup DL(CurFileEnt->getDir(), IncludingHFI.getDirInfo(),
+ !IncludingHFI.Framework.empty());
+ if (const FileEntry *FE =
+ DL.LookupFile(Filename, *this, SearchPath, RelativePath,
+ SuggestedModule, InUserSpecifiedSystemFramework)) {
// 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(CurFileEnt);
- unsigned DirInfo = FromHFI.DirInfo;
+ SrcMgr::CharacteristicKind DirInfo = FromHFI.getDirInfo();
bool IndexHeaderMapHeader = FromHFI.IndexHeaderMapHeader;
StringRef Framework = FromHFI.Framework;
HeaderFileInfo &ToHFI = getFileInfo(FE);
ToHFI.DirInfo = DirInfo;
ToHFI.IndexHeaderMapHeader = IndexHeaderMapHeader;
ToHFI.Framework = Framework;
- if (SearchPath != NULL) {
- StringRef SearchPathRef(CurFileEnt->getDir()->getName());
- SearchPath->clear();
- SearchPath->append(SearchPathRef.begin(), SearchPathRef.end());
- }
- if (RelativePath != NULL) {
- RelativePath->clear();
- RelativePath->append(Filename.begin(), Filename.end());
- }
+ // If the directory characteristic is User but this framework was
+ // user-specified to be treated as a system framework, promote the
+ // characteristic.
+ if (ToHFI.DirInfo == SrcMgr::C_User && InUserSpecifiedSystemFramework)
+ ToHFI.DirInfo = SrcMgr::C_System;
+
return FE;
}
}
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -613,10 +613,10 @@
<< RequestingModule->getFullModuleName() << Filename;
}
-const FileEntry *Preprocessor::LookupFile(
+const FileEntry *Preprocessor::lookupFileImpl(
SourceLocation FilenameLoc,
StringRef Filename,
- bool isAngled,
+ bool IsAngled,
const DirectoryLookup *FromDir,
const DirectoryLookup *&CurDir,
SmallVectorImpl<char> *SearchPath,
@@ -645,13 +645,10 @@
// Do a standard file entry lookup.
CurDir = CurDirLookup;
const FileEntry *FE = HeaderInfo.LookupFile(
- Filename, isAngled, FromDir, CurDir, CurFileEnt,
+ Filename, IsAngled, FromDir, CurDir, CurFileEnt,
SearchPath, RelativePath, SuggestedModule, SkipCache);
- if (FE) {
- if (SuggestedModule)
- verifyModuleInclude(FilenameLoc, Filename, FE);
+ if (FE)
return FE;
- }
// Otherwise, see if this is a subframework header. If so, this is relative
// to one of the headers on the #include stack. Walk the list of the current
@@ -680,6 +677,31 @@
return 0;
}
+const FileEntry *Preprocessor::LookupFile(
+ SourceLocation FilenameLoc,
+ StringRef Filename,
+ bool IsAngled,
+ const DirectoryLookup *FromDir,
+ const DirectoryLookup *&CurDir,
+ SmallVectorImpl<char> *SearchPath,
+ SmallVectorImpl<char> *RelativePath,
+ ModuleMap::KnownHeader *SuggestedModule,
+ bool SkipCache) {
+ const FileEntry *FE =
+ lookupFileImpl(FilenameLoc, Filename, IsAngled, FromDir, CurDir,
+ SearchPath, RelativePath, SuggestedModule, SkipCache);
+ if (FE && SuggestedModule)
+ verifyModuleInclude(FilenameLoc, Filename, FE);
+ return FE;
+}
+
+const FileEntry *Preprocessor::lookupFileForDiagnostics(
+ SourceLocation FilenameLoc, StringRef Filename, bool IsAngled) {
+ const DirectoryLookup *CurDir;
+ return lookupFileImpl(FilenameLoc, Filename, IsAngled, NULL, CurDir,
+ NULL, NULL, NULL, false);
+}
+
//===----------------------------------------------------------------------===//
// Preprocessor Directive Handling.
Index: test/Modules/Inputs/declare-use/h.h
===================================================================
--- test/Modules/Inputs/declare-use/h.h
+++ test/Modules/Inputs/declare-use/h.h
@@ -1,7 +1,7 @@
#ifndef H_H
#define H_H
#include "c.h"
-#include "d.h" // expected-error {{does not depend on a module exporting}}
+#include "d.h" // expected-error {{module XH does not depend on a module exporting 'd.h'}}
#include "h1.h"
const int h1 = aux_h*c*7*d;
#endif
Index: test/Modules/Inputs/declare-use/module.map
===================================================================
--- test/Modules/Inputs/declare-use/module.map
+++ test/Modules/Inputs/declare-use/module.map
@@ -9,11 +9,13 @@
module XC {
header "c.h"
use XA
+ use XB
}
module XD {
header "d.h"
use XA
+ use XB
}
module XE {
Index: test/Modules/declare-use2.cpp
===================================================================
--- test/Modules/declare-use2.cpp
+++ test/Modules/declare-use2.cpp
@@ -3,5 +3,5 @@
#include "h.h"
#include "e.h"
-#include "f.h" // expected-error {{does not depend on a module exporting}}
+#include "f.h" // expected-error {{module XH does not depend on a module exporting 'f.h'}}
const int h2 = h1+e+f;
Index: test/Modules/submodules.cpp
===================================================================
--- test/Modules/submodules.cpp
+++ test/Modules/submodules.cpp
@@ -31,6 +31,6 @@
extern MyTypeA import_self_test_a; // expected-error {{must be imported from module 'import_self.a'}}
// [email protected]: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'}}
-// [email protected]:1 {{here}}
+
+// import_self.b re-exports import_self.d.
+extern MyTypeD import_self_test_d;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
