[PATCH] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

Alex Hoppen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 14 09:26:49 PST 2022


ahoppen added inline comments.


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:710
+  CacheLookup.HitIt = HitIt;
+  noteLookupUsage(&*HitIt - &*search_dir_begin(), Loc);
 }
----------------
I haven’t looked into this in total details but `&*` looks a little awkward to me. Dereference a pointer/iteration and then get its pointer value back? 

Wouldn’t this hit the same issue that we saw before if `serach_dir_begin` is allocated in a different bump allocator begin than `HitIt`?

If possible, would `std::distance` communicate the intent more clearly?


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:982
 
+  ConstSearchDirIterator NextIt = [](auto ItCopy) { return ++ItCopy; }(It);
+
----------------
What’s the reason that this can’t be? The current lambda-based implementation looks a little over-complicated to me. But maybe I’m missing something.
```
ConstSearchDirIterator NextIt = ItCopy;
++NextIt;
```

or even something equivalent to 
```
ConstSearchDirIterator NextIt = std::next(ItCopy);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119721



More information about the cfe-commits mailing list