[clang][lex] Introduce ConstSearchDirIterator

The `const DirectoryLookup *` out-parameter of `{HeaderSearch,Preprocessor}::LookupFile()` is assigned the most recently used search directory, which callers use to implement `#include_next`.

From the function signature it's not obvious the `const DirectoryLookup *` is being used as an iterator. This patch introduces `ConstSearchDirIterator` to make that affordance obvious. This would've prevented a bug that occurred after initially landing D116750.

Reviewed By: ahoppen

Differential Revision: https://reviews.llvm.org/D117566

clang/include/clang/Lex/HeaderSearch.h

@@ -47,6 +47,7 @@ class DirectoryEntry;
 class ExternalPreprocessorSource;
 class FileEntry;
 class FileManager;
+class HeaderSearch;
 class HeaderSearchOptions;
 class IdentifierInfo;
 class LangOptions;
@@ -162,11 +163,55 @@ struct FrameworkCacheEntry {
   bool IsUserSpecifiedSystemFramework;
 };
 
+/// Forward iterator over the search directories of \c HeaderSearch.
+struct ConstSearchDirIterator
+    : llvm::iterator_facade_base<ConstSearchDirIterator,
+                                 std::forward_iterator_tag,
+                                 const DirectoryLookup> {
+  ConstSearchDirIterator(const ConstSearchDirIterator &) = default;
+
+  ConstSearchDirIterator &operator=(const ConstSearchDirIterator &) = default;
+
+  bool operator==(const ConstSearchDirIterator &RHS) const {
+    return HS == RHS.HS && Idx == RHS.Idx;
+  }
+
+  ConstSearchDirIterator &operator++() {
+    assert(*this && "Invalid iterator.");
+    ++Idx;
+    return *this;
+  }
+
+  const DirectoryLookup &operator*() const;
+
+  /// Creates an invalid iterator.
+  ConstSearchDirIterator(std::nullptr_t) : HS(nullptr), Idx(0) {}
+
+  /// Checks whether the iterator is valid.
+  explicit operator bool() const { return HS != nullptr; }
+
+private:
+  /// The parent \c HeaderSearch. This is \c nullptr for invalid iterator.
+  const HeaderSearch *HS;
+
+  /// The index of the current element.
+  size_t Idx;
+
+  /// The constructor that creates a valid iterator.
+  ConstSearchDirIterator(const HeaderSearch &HS, size_t Idx)
+      : HS(&HS), Idx(Idx) {}
+
+  /// Only HeaderSearch is allowed to instantiate valid iterators.
+  friend HeaderSearch;
+};
+
 /// Encapsulates the information needed to find the file referenced
 /// by a \#include or \#include_next, (sub-)framework lookup, etc.
 class HeaderSearch {
   friend class DirectoryLookup;
 
+  friend ConstSearchDirIterator;
+
   /// Header-search options used to initialize this header search.
   std::shared_ptr<HeaderSearchOptions> HSOpts;
 
@@ -407,7 +452,7 @@ class HeaderSearch {
   /// found.
   Optional<FileEntryRef> LookupFile(
       StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
-      const DirectoryLookup *FromDir, const DirectoryLookup **CurDir,
+      ConstSearchDirIterator FromDir, ConstSearchDirIterator *CurDir,
       ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
       SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
       Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
@@ -731,35 +776,26 @@ class HeaderSearch {
   const HeaderFileInfo *getExistingFileInfo(const FileEntry *FE,
                                             bool WantExternal = true) const;
 
-  // Used by external tools
-  using search_dir_iterator = std::vector<DirectoryLookup>::const_iterator;
+  ConstSearchDirIterator search_dir_begin() const { return quoted_dir_begin(); }
+  ConstSearchDirIterator search_dir_end() const { return system_dir_end(); }
 
-  search_dir_iterator search_dir_begin() const { return SearchDirs.begin(); }
-  search_dir_iterator search_dir_end() const { return SearchDirs.end(); }
   unsigned search_dir_size() const { return SearchDirs.size(); }
 
-  search_dir_iterator quoted_dir_begin() const {
-    return SearchDirs.begin();
-  }
+  ConstSearchDirIterator quoted_dir_begin() const { return {*this, 0}; }
+  ConstSearchDirIterator quoted_dir_end() const { return angled_dir_begin(); }
 
-  search_dir_iterator quoted_dir_end() const {
-    return SearchDirs.begin() + AngledDirIdx;
+  ConstSearchDirIterator angled_dir_begin() const {
+    return {*this, AngledDirIdx};
   }
+  ConstSearchDirIterator angled_dir_end() const { return system_dir_begin(); }
 
-  search_dir_iterator angled_dir_begin() const {
-    return SearchDirs.begin() + AngledDirIdx;
+  ConstSearchDirIterator system_dir_begin() const {
+    return {*this, SystemDirIdx};
   }
-
-  search_dir_iterator angled_dir_end() const {
-    return SearchDirs.begin() + SystemDirIdx;
+  ConstSearchDirIterator system_dir_end() const {
+    return {*this, SearchDirs.size()};
   }
 
-  search_dir_iterator system_dir_begin() const {
-    return SearchDirs.begin() + SystemDirIdx;
-  }
-
-  search_dir_iterator system_dir_end() const { return SearchDirs.end(); }
-
   /// Get the index of the given search directory.
   Optional<unsigned> searchDirIdx(const DirectoryLookup &DL) const;
 

clang/include/clang/Lex/Preprocessor.h

@@ -517,7 +517,7 @@ class Preprocessor {
   ///
   /// This allows us to implement \#include_next and find directory-specific
   /// properties.
-  const DirectoryLookup *CurDirLookup = nullptr;
+  ConstSearchDirIterator CurDirLookup = nullptr;
 
   /// The current macro we are expanding, if we are expanding a macro.
   ///
@@ -545,15 +545,15 @@ class Preprocessor {
     std::unique_ptr<Lexer>      TheLexer;
     PreprocessorLexer          *ThePPLexer;
     std::unique_ptr<TokenLexer> TheTokenLexer;
-    const DirectoryLookup      *TheDirLookup;
+    ConstSearchDirIterator      TheDirLookup;
 
     // The following constructors are completely useless copies of the default
     // versions, only needed to pacify MSVC.
     IncludeStackInfo(enum CurLexerKind CurLexerKind, Module *TheSubmodule,
                      std::unique_ptr<Lexer> &&TheLexer,
                      PreprocessorLexer *ThePPLexer,
                      std::unique_ptr<TokenLexer> &&TheTokenLexer,
-                     const DirectoryLookup *TheDirLookup)
+                     ConstSearchDirIterator TheDirLookup)
         : CurLexerKind(std::move(CurLexerKind)),
           TheSubmodule(std::move(TheSubmodule)), TheLexer(std::move(TheLexer)),
           ThePPLexer(std::move(ThePPLexer)),
@@ -1388,7 +1388,7 @@ class Preprocessor {
   /// start lexing tokens from it instead of the current buffer.
   ///
   /// Emits a diagnostic, doesn't enter the file, and returns true on error.
-  bool EnterSourceFile(FileID FID, const DirectoryLookup *Dir,
+  bool EnterSourceFile(FileID FID, ConstSearchDirIterator Dir,
                        SourceLocation Loc, bool IsFirstIncludeOfFile = true);
 
   /// Add a Macro to the top of the include stack and start lexing
@@ -2071,8 +2071,8 @@ class Preprocessor {
   /// reference is for system \#include's or not (i.e. using <> instead of "").
   Optional<FileEntryRef>
   LookupFile(SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
-             const DirectoryLookup *FromDir, const FileEntry *FromFile,
-             const DirectoryLookup **CurDir, SmallVectorImpl<char> *SearchPath,
+             ConstSearchDirIterator FromDir, const FileEntry *FromFile,
+             ConstSearchDirIterator *CurDir, SmallVectorImpl<char> *SearchPath,
              SmallVectorImpl<char> *RelativePath,
              ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
              bool *IsFrameworkFound, bool SkipCache = false);
@@ -2201,7 +2201,7 @@ class Preprocessor {
   bool EvaluateHasIncludeNext(Token &Tok, IdentifierInfo *II);
 
   /// Get the directory and file from which to start \#include_next lookup.
-  std::pair<const DirectoryLookup *, const FileEntry *>
+  std::pair<ConstSearchDirIterator, const FileEntry *>
   getIncludeNextStart(const Token &IncludeNextTok) const;
 
   /// Install the standard preprocessor pragmas:
@@ -2251,7 +2251,7 @@ class Preprocessor {
 
   /// Add a lexer to the top of the include stack and
   /// start lexing tokens from it instead of the current buffer.
-  void EnterSourceFileWithLexer(Lexer *TheLexer, const DirectoryLookup *Dir);
+  void EnterSourceFileWithLexer(Lexer *TheLexer, ConstSearchDirIterator Dir);
 
   /// Set the FileID for the preprocessor predefines.
   void setPredefinesFileID(FileID FID) {
@@ -2328,22 +2328,22 @@ class Preprocessor {
   };
 
   Optional<FileEntryRef> LookupHeaderIncludeOrImport(
-      const DirectoryLookup **CurDir, StringRef &Filename,
+      ConstSearchDirIterator *CurDir, StringRef &Filename,
       SourceLocation FilenameLoc, CharSourceRange FilenameRange,
       const Token &FilenameTok, bool &IsFrameworkFound, bool IsImportDecl,
-      bool &IsMapped, const DirectoryLookup *LookupFrom,
+      bool &IsMapped, ConstSearchDirIterator LookupFrom,
       const FileEntry *LookupFromFile, StringRef &LookupFilename,
       SmallVectorImpl<char> &RelativePath, SmallVectorImpl<char> &SearchPath,
       ModuleMap::KnownHeader &SuggestedModule, bool isAngled);
 
   // File inclusion.
   void HandleIncludeDirective(SourceLocation HashLoc, Token &Tok,
-                              const DirectoryLookup *LookupFrom = nullptr,
+                              ConstSearchDirIterator LookupFrom = nullptr,
                               const FileEntry *LookupFromFile = nullptr);
   ImportAction
   HandleHeaderIncludeOrImport(SourceLocation HashLoc, Token &IncludeTok,
                               Token &FilenameTok, SourceLocation EndLoc,
-                              const DirectoryLookup *LookupFrom = nullptr,
+                              ConstSearchDirIterator LookupFrom = nullptr,
                               const FileEntry *LookupFromFile = nullptr);
   void HandleIncludeNextDirective(SourceLocation HashLoc, Token &Tok);
   void HandleIncludeMacrosDirective(SourceLocation HashLoc, Token &Tok);

clang/lib/Lex/HeaderSearch.cpp

@@ -79,6 +79,11 @@ HeaderFileInfo::getControllingMacro(ExternalPreprocessorSource *External) {
 
 ExternalHeaderFileInfoSource::~ExternalHeaderFileInfoSource() = default;
 
+const DirectoryLookup &ConstSearchDirIterator::operator*() const {
+  assert(*this && "Invalid iterator.");
+  return HS->SearchDirs[Idx];
+}
+
 HeaderSearch::HeaderSearch(std::shared_ptr<HeaderSearchOptions> HSOpts,
                            SourceManager &SourceMgr, DiagnosticsEngine &Diags,
                            const LangOptions &LangOpts,
@@ -829,14 +834,14 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc,
 /// search is needed. Microsoft mode will pass all \#including files.
 Optional<FileEntryRef> HeaderSearch::LookupFile(
     StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
-    const DirectoryLookup *FromDir, const DirectoryLookup **CurDirArg,
+    ConstSearchDirIterator FromDir, ConstSearchDirIterator *CurDirArg,
     ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
     SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
     Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
     bool *IsMapped, bool *IsFrameworkFound, bool SkipCache,
     bool BuildSystemModule) {
-  const DirectoryLookup *CurDirLocal = nullptr;
-  const DirectoryLookup *&CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
+  ConstSearchDirIterator CurDirLocal = nullptr;
+  ConstSearchDirIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
 
   if (IsMapped)
     *IsMapped = false;
@@ -964,7 +969,7 @@ Optional<FileEntryRef> HeaderSearch::LookupFile(
   // If this is a #include_next request, start searching after the directory the
   // file was found in.
   if (FromDir)
-    i = FromDir-&SearchDirs[0];
+    i = FromDir.Idx;
 
   // Cache all of the lookups performed by this method.  Many headers are
   // multiply included, and the "pragma once" optimization prevents them from
@@ -1019,7 +1024,7 @@ Optional<FileEntryRef> HeaderSearch::LookupFile(
     if (!File)
       continue;
 
-    CurDir = &SearchDirs[i];
+    CurDir = ConstSearchDirIterator(*this, i);
 
     // This file is a system header or C++ unfriendly if the dir is.
     HeaderFileInfo &HFI = getFileInfo(&File->getFileEntry());

clang/lib/Lex/PPDirectives.cpp

@@ -817,13 +817,13 @@ Preprocessor::getHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
 
 Optional<FileEntryRef> Preprocessor::LookupFile(
     SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
-    const DirectoryLookup *FromDir, const FileEntry *FromFile,
-    const DirectoryLookup **CurDirArg, SmallVectorImpl<char> *SearchPath,
+    ConstSearchDirIterator FromDir, const FileEntry *FromFile,
+    ConstSearchDirIterator *CurDirArg, SmallVectorImpl<char> *SearchPath,
     SmallVectorImpl<char> *RelativePath,
     ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
     bool *IsFrameworkFound, bool SkipCache) {
-  const DirectoryLookup *CurDirLocal = nullptr;
-  const DirectoryLookup *&CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
+  ConstSearchDirIterator CurDirLocal = nullptr;
+  ConstSearchDirIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
 
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
@@ -877,8 +877,8 @@ Optional<FileEntryRef> Preprocessor::LookupFile(
   if (FromFile) {
     // We're supposed to start looking from after a particular file. Search
     // the include path until we find that file or run out of files.
-    const DirectoryLookup *TmpCurDir = CurDir;
-    const DirectoryLookup *TmpFromDir = nullptr;
+    ConstSearchDirIterator TmpCurDir = CurDir;
+    ConstSearchDirIterator TmpFromDir = nullptr;
     while (Optional<FileEntryRef> FE = HeaderInfo.LookupFile(
                Filename, FilenameLoc, isAngled, TmpFromDir, &TmpCurDir,
                Includers, SearchPath, RelativePath, RequestingModule,
@@ -1778,12 +1778,12 @@ bool Preprocessor::checkModuleIsAvailable(const LangOptions &LangOpts,
   return true;
 }
 
-std::pair<const DirectoryLookup *, const FileEntry *>
+std::pair<ConstSearchDirIterator, const FileEntry *>
 Preprocessor::getIncludeNextStart(const Token &IncludeNextTok) const {
   // #include_next is like #include, except that we start searching after
   // the current found directory.  If we can't do this, issue a
   // diagnostic.
-  const DirectoryLookup *Lookup = CurDirLookup;
+  ConstSearchDirIterator Lookup = CurDirLookup;
   const FileEntry *LookupFromFile = nullptr;
 
   if (isInPrimaryFile() && LangOpts.IsHeaderFile) {
@@ -1820,7 +1820,7 @@ Preprocessor::getIncludeNextStart(const Token &IncludeNextTok) const {
 /// specifies the file to start searching from.
 void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
                                           Token &IncludeTok,
-                                          const DirectoryLookup *LookupFrom,
+                                          ConstSearchDirIterator LookupFrom,
                                           const FileEntry *LookupFromFile) {
   Token FilenameTok;
   if (LexHeaderName(FilenameTok))
@@ -1865,11 +1865,11 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
 }
 
 Optional<FileEntryRef> Preprocessor::LookupHeaderIncludeOrImport(
-    const DirectoryLookup **CurDir, StringRef& Filename,
+    ConstSearchDirIterator *CurDir, StringRef &Filename,
     SourceLocation FilenameLoc, CharSourceRange FilenameRange,
     const Token &FilenameTok, bool &IsFrameworkFound, bool IsImportDecl,
-    bool &IsMapped, const DirectoryLookup *LookupFrom,
-    const FileEntry *LookupFromFile, StringRef& LookupFilename,
+    bool &IsMapped, ConstSearchDirIterator LookupFrom,
+    const FileEntry *LookupFromFile, StringRef &LookupFilename,
     SmallVectorImpl<char> &RelativePath, SmallVectorImpl<char> &SearchPath,
     ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
   Optional<FileEntryRef> File = LookupFile(
@@ -1973,7 +1973,7 @@ Optional<FileEntryRef> Preprocessor::LookupHeaderIncludeOrImport(
 ///        lookup.
 Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
     SourceLocation HashLoc, Token &IncludeTok, Token &FilenameTok,
-    SourceLocation EndLoc, const DirectoryLookup *LookupFrom,
+    SourceLocation EndLoc, ConstSearchDirIterator LookupFrom,
     const FileEntry *LookupFromFile) {
   SmallString<128> FilenameBuffer;
   StringRef Filename = getSpelling(FilenameTok, FilenameBuffer);
@@ -2023,7 +2023,7 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
   // Search include directories.
   bool IsMapped = false;
   bool IsFrameworkFound = false;
-  const DirectoryLookup *CurDir;
+  ConstSearchDirIterator CurDir = nullptr;
   SmallString<1024> SearchPath;
   SmallString<1024> RelativePath;
   // We get the raw path only if we have 'Callbacks' to which we later pass
@@ -2410,7 +2410,7 @@ void Preprocessor::HandleIncludeNextDirective(SourceLocation HashLoc,
                                               Token &IncludeNextTok) {
   Diag(IncludeNextTok, diag::ext_pp_include_next_directive);
 
-  const DirectoryLookup *Lookup;
+  ConstSearchDirIterator Lookup = nullptr;
   const FileEntry *LookupFromFile;
   std::tie(Lookup, LookupFromFile) = getIncludeNextStart(IncludeNextTok);
 

clang/lib/Lex/PPLexerChange.cpp

@@ -66,7 +66,7 @@ PreprocessorLexer *Preprocessor::getCurrentFileLexer() const {
 
 /// EnterSourceFile - Add a source file to the top of the include stack and
 /// start lexing tokens from it instead of the current buffer.
-bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
+bool Preprocessor::EnterSourceFile(FileID FID, ConstSearchDirIterator CurDir,
                                    SourceLocation Loc,
                                    bool IsFirstIncludeOfFile) {
   assert(!CurTokenLexer && "Cannot #include a file inside a macro!");
@@ -100,7 +100,7 @@ bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
 /// EnterSourceFileWithLexer - Add a source file to the top of the include stack
 ///  and start lexing tokens from it instead of the current buffer.
 void Preprocessor::EnterSourceFileWithLexer(Lexer *TheLexer,
-                                            const DirectoryLookup *CurDir) {
+                                            ConstSearchDirIterator CurDir) {
 
   // Add the current lexer to the include stack.
   if (CurPPLexer || CurTokenLexer)

clang/lib/Lex/PPMacroExpansion.cpp

@@ -1157,9 +1157,9 @@ static bool HasExtension(const Preprocessor &PP, StringRef Extension) {
 /// EvaluateHasIncludeCommon - Process a '__has_include("path")'
 /// or '__has_include_next("path")' expression.
 /// Returns true if successful.
-static bool EvaluateHasIncludeCommon(Token &Tok,
-                                     IdentifierInfo *II, Preprocessor &PP,
-                                     const DirectoryLookup *LookupFrom,
+static bool EvaluateHasIncludeCommon(Token &Tok, IdentifierInfo *II,
+                                     Preprocessor &PP,
+                                     ConstSearchDirIterator LookupFrom,
                                      const FileEntry *LookupFromFile) {
   // Save the location of the current token.  If a '(' is later found, use
   // that location.  If not, use the end of this location instead.
@@ -1249,7 +1249,7 @@ bool Preprocessor::EvaluateHasInclude(Token &Tok, IdentifierInfo *II) {
 }
 
 bool Preprocessor::EvaluateHasIncludeNext(Token &Tok, IdentifierInfo *II) {
-  const DirectoryLookup *Lookup;
+  ConstSearchDirIterator Lookup = nullptr;
   const FileEntry *LookupFromFile;
   std::tie(Lookup, LookupFromFile) = getIncludeNextStart(Tok);