[PATCH] D116750: [clang][lex] Keep references to `DirectoryLookup` objects up-to-date

Tue Jan 18 08:05:06 PST 2022

jansvoboda11 added inline comments.

================
Comment at: clang/include/clang/Lex/HeaderSearch.h:415
       StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
-      const DirectoryLookup *FromDir, const DirectoryLookup **CurDir,
+      maybe_search_dir_iterator FromDir, maybe_search_dir_iterator *CurDir,
       ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
----------------
jansvoboda11 wrote:
> This is the interesting change in the latest revision. Callers of this function performed pointer arithmetics on `const DirectoryLookup *`, essentially treating it as an iterator. Since `DirectoryLookup` objects are no longer stored in contiguous memory, that's unsafe. I'm using an iterator to allow doing that safely and using `llvm::Optional` to provide the nullability clients take advantage of.
> 
> I don't love the long name that causes a bunch of formatting changes. Maybe it's worth extracting it out of `HeaderSearch` and giving it shorter name. I also don't love that the API is not the same as for `const DirectoryLookup *`, leading to some "unnecessary" changes (e.g. `CurDir = nullptr` -> `CurDir.reset()`). I think it might be worth creating custom iterator type that would have the original API, but would behave correctly with the new memory setup.
Resolved in the latest revision by introducing D117566.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116750/new/

https://reviews.llvm.org/D116750