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

Jan Svoboda via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 14 07:04:47 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,
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750



More information about the cfe-commits mailing list