[PATCH] D66782: SourceManager: Prefer Optional<MemoryBufferRef> over MemoryBuffer*

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

dexonsmith planned changes to this revision.
dexonsmith added a comment.

I'm going to change my approach here to be less error-prone. The real goal is changing from `MemoryBuffer*` to `MemoryBufferRef`. I was adding `Optional<>` as a cleanup but that's really a nice-to-have, which is better landed independently.

1. Add getBufferOrNone and getBufferDataOrNone, which return `Optional<MemoryBufferRef>`.
2. Move users of getBuffer and getBufferData that use the `bool* Invalid` out parameter over to that API.
3. (this patch) Change getBuffer and getBufferData to return `MemoryBufferRef`, dropping the (now unused) Invalid parameter. Keep the semantics where they return a fake buffer if the real one couldn't be found.
4. (optional, later) Rename getBuffer to  getBufferOrFake (same for Data)
5. (optional, later) Migrate audited APIs over to a new/clean getBuffer API that asserts on invalid/None

Compare to current approach in the patch:

- good: callers will still get expected results when invalid (for those that don’t check); no audit necessary
- good: getBuffer and getBufferData will match
- good: more incremental
- bad: a bit more churn for getBuffer users, since pointer semantics (`MemoryBuffer*`) change to reference semantics (`MemoryBufferRef`)
- bad: delay cleanup where callers expecting fake data explicitly use an `OrFake` API


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

https://reviews.llvm.org/D66782