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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to