From b5d8fba05e74d73ef40056b8fe4b4a611e02a728 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 11 Mar 2014 06:21:28 +0000 Subject: [PATCH] [HeaderSearch] Fix issue where if a headermap entry maps the filename to a framework import (non-absolute path) then we fail to find it if it is re-included later on. rdar://16285490 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@203542 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Lex/DirectoryLookup.h | 1 + include/clang/Lex/HeaderSearch.h | 22 ++++++++++------- lib/Lex/HeaderSearch.cpp | 38 +++++++++++++++++++++-------- test/Preprocessor/headermap-rel.c | 2 ++ 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/include/clang/Lex/DirectoryLookup.h b/include/clang/Lex/DirectoryLookup.h index 89939c84ca..16899a074b 100644 --- a/include/clang/Lex/DirectoryLookup.h +++ b/include/clang/Lex/DirectoryLookup.h @@ -170,6 +170,7 @@ public: SmallVectorImpl *RelativePath, ModuleMap::KnownHeader *SuggestedModule, bool &InUserSpecifiedSystemFramework, + bool &HasBeenMapped, SmallVectorImpl &MappedName) const; private: diff --git a/include/clang/Lex/HeaderSearch.h b/include/clang/Lex/HeaderSearch.h index 796ea3fec1..d1cb8118c0 100644 --- a/include/clang/Lex/HeaderSearch.h +++ b/include/clang/Lex/HeaderSearch.h @@ -186,15 +186,19 @@ class HeaderSearch { /// included, indexed by the FileEntry's UID. std::vector FileInfo; - /// \brief Keeps track of each lookup performed by LookupFile. - /// - /// The first part of the value is the starting index in SearchDirs - /// that the cached search was performed from. If there is a hit and - /// this value doesn't match the current query, the cache has to be - /// ignored. The second value is the entry in SearchDirs that satisfied - /// the query. - llvm::StringMap, llvm::BumpPtrAllocator> - LookupFileCache; + /// Keeps track of each lookup performed by LookupFile. + struct LookupFileCacheInfo { + /// Starting index in SearchDirs that the cached search was performed from. + /// If there is a hit and this value doesn't match the current query, the + /// cache has to be ignored. + unsigned StartIdx = 0; + /// The entry in SearchDirs that satisfied the query. + unsigned HitIdx = 0; + /// This is non-null if the original filename was mapped to a framework + /// include via a headermap. + const char *MappedName = nullptr; + }; + llvm::StringMap LookupFileCache; /// \brief Collection mapping a framework or subframework /// name like "Carbon" to the Carbon.framework directory. diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp index 3c5eb0ead8..97bff4f277 100644 --- a/lib/Lex/HeaderSearch.cpp +++ b/lib/Lex/HeaderSearch.cpp @@ -255,8 +255,10 @@ const FileEntry *DirectoryLookup::LookupFile( SmallVectorImpl *RelativePath, ModuleMap::KnownHeader *SuggestedModule, bool &InUserSpecifiedSystemFramework, + bool &HasBeenMapped, SmallVectorImpl &MappedName) const { InUserSpecifiedSystemFramework = false; + HasBeenMapped = false; SmallString<1024> TmpDir; if (isNormalDir()) { @@ -298,6 +300,7 @@ const FileEntry *DirectoryLookup::LookupFile( MappedName.clear(); MappedName.append(Dest.begin(), Dest.end()); Filename = StringRef(MappedName.begin(), MappedName.size()); + HasBeenMapped = true; Result = HM->LookupFile(Filename, HS.getFileMgr()); } else { @@ -533,6 +536,14 @@ static bool checkMSVCHeaderSearch(DiagnosticsEngine &Diags, return false; } +static const char *copyString(StringRef Str, llvm::BumpPtrAllocator &Alloc) { + assert(!Str.empty()); + char *CopyStr = Alloc.Allocate(Str.size()+1); + std::copy(Str.begin(), Str.end(), CopyStr); + CopyStr[Str.size()] = '\0'; + return CopyStr; +} + /// LookupFile - Given a "foo" or \ reference, look up the indicated file, /// return null on failure. isAngled indicates whether the file reference is /// for system \#include's or not (i.e. using <> instead of ""). Includers, if @@ -673,20 +684,22 @@ const FileEntry *HeaderSearch::LookupFile( // multiply included, and the "pragma once" optimization prevents them from // being relex/pp'd, but they would still have to search through a // (potentially huge) series of SearchDirs to find it. - std::pair &CacheLookup = + LookupFileCacheInfo &CacheLookup = LookupFileCache.GetOrCreateValue(Filename).getValue(); // If the entry has been previously looked up, the first value will be // non-zero. If the value is equal to i (the start point of our search), then // this is a matching hit. - if (!SkipCache && CacheLookup.first == i+1) { + if (!SkipCache && CacheLookup.StartIdx == i+1) { // Skip querying potentially lots of directories for this lookup. - i = CacheLookup.second; + i = CacheLookup.HitIdx; + if (CacheLookup.MappedName) + Filename = CacheLookup.MappedName; } else { // Otherwise, this is the first query, or the previous query didn't match // our search start. We will fill in our found location below, so prime the // start point value. - CacheLookup.first = i+1; + CacheLookup.StartIdx = i+1; } SmallString<64> MappedName; @@ -694,10 +707,15 @@ const FileEntry *HeaderSearch::LookupFile( // Check each directory in sequence to see if it contains this file. for (; i != SearchDirs.size(); ++i) { bool InUserSpecifiedSystemFramework = false; + bool HasBeenMapped = false; const FileEntry *FE = SearchDirs[i].LookupFile(Filename, *this, SearchPath, RelativePath, SuggestedModule, InUserSpecifiedSystemFramework, - MappedName); + HasBeenMapped, MappedName); + if (HasBeenMapped) { + CacheLookup.MappedName = + copyString(Filename, LookupFileCache.getAllocator()); + } if (!FE) continue; CurDir = &SearchDirs[i]; @@ -740,7 +758,7 @@ const FileEntry *HeaderSearch::LookupFile( } // Remember this location for the next lookup we do. - CacheLookup.second = i; + CacheLookup.HitIdx = i; return FE; } @@ -767,10 +785,10 @@ const FileEntry *HeaderSearch::LookupFile( return MSFE; } - std::pair &CacheLookup + LookupFileCacheInfo &CacheLookup = LookupFileCache.GetOrCreateValue(Filename).getValue(); - CacheLookup.second - = LookupFileCache.GetOrCreateValue(ScratchFilename).getValue().second; + CacheLookup.HitIdx + = LookupFileCache.GetOrCreateValue(ScratchFilename).getValue().HitIdx; // FIXME: SuggestedModule. return FE; } @@ -783,7 +801,7 @@ const FileEntry *HeaderSearch::LookupFile( } // Otherwise, didn't find it. Remember we didn't find this. - CacheLookup.second = SearchDirs.size(); + CacheLookup.HitIdx = SearchDirs.size(); return 0; } diff --git a/test/Preprocessor/headermap-rel.c b/test/Preprocessor/headermap-rel.c index a3b11218d5..38500a70f6 100644 --- a/test/Preprocessor/headermap-rel.c +++ b/test/Preprocessor/headermap-rel.c @@ -5,6 +5,8 @@ // RUN: %clang_cc1 -E %s -o %t.i -I %S/Inputs/headermap-rel/foo.hmap -F %S/Inputs/headermap-rel // RUN: FileCheck %s -input-file %t.i +// CHECK: Foo.h is parsed // CHECK: Foo.h is parsed #include "Foo.h" +#include "Foo.h" -- 2.40.0