rsmith added a comment.

In D132779#4523783 <https://reviews.llvm.org/D132779#4523783>, @jansvoboda11 
wrote:

> Hi @rsmith, this commit makes it possible for `HeaderInfo::LookupFile()` to 
> be called with different `RequestingModule` within single `CompilerInstance`. 
> This is problematic, since some modules may see headers other modules can't 
> (due to `[no_undeclared_includes]`). This can permanently mess up contents of 
> the lookup cache (`HeaderSearch::LookupFileCache`) that uses only the lookup 
> name as the key.

Hm, interesting. I wonder if this was (subtly) broken before, given that it was 
previously possible to call `LookupFile` with different `RequestingModule`s in 
a single compilation -- once with a null `RequestingModule` and once with the 
compilation's current module. In any case, presumably we should either ensure 
that the cached data is independent of the requesting module (eg, apply the 
decluse restriction after computing the iterator pair), or add the requesting 
module to the key, or only cache data for one specific value of 
`RequestingModule`. Maybe the simplest thing would be to use a single-item LRU 
cache here -- store the `RequestingModule` in the cached result and ignore the 
existing cached result if it doesn't match the current `RequestingModule`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132779

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D132779: ... Jan Svoboda via Phabricator via cfe-commits
    • [PATCH] D132... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to