[PATCH] D89834: FileManager: Improve the FileEntryRef API and add MaybeFileEntryRef

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D89834#2356769 <https://reviews.llvm.org/D89834#2356769>, @arphaman wrote:

> What's wrong with using `Optional<FileEntryRef>` instead of `MaybeFileEntryRef`?

Two problems:

1. `const FileEntry*` is stored in lots of places. I am nervous after the memory regression from `FileEntryRef`'s introduction (https://reviews.llvm.org/D89580) that doubling the size of these fields will matter. I'd rather save it.

2. As mentioned in the description, the "degrade to `const FileEntry *`" behaviour greatly reduces the code churn for incremental patches.

I realize in retrospect I can fix the first problem by customizing `OptionalStorage<FileEntryRef>`.

IIRC, the follow-up patches got significantly (2x? 5x? it was a lot) smaller when I fixed the second problem. I'm not sure if it's worth it.


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

https://reviews.llvm.org/D89834