[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

@steakhal Could you review this change when you have some time for it?

I'm planning to stabilize and de-alpha this checker with a series of incremental changes; and it'd be good have this commit and the unrelated tweak https://reviews.llvm.org/D149460 merged before I start working on the next steps. My plans for the near future:

- Replace the prototype-level error reporting with real, detailed, useful messages. Implementing this may require some code reorganization to preserve and pass around the details information.
- Perform an early return when there is no real indexing (there are no ElementRegion layers in the load operation).
- Reconsider the handling of multidimensional arrays: the current code (in fact the function `computeOffset` that I'm refactoring there) says that `arr[0][10]` is a valid way to index `int arr[4][5]` which not what the users would expect (in this simple case, even clang-tidy knows that "array index 10 is past the end of the array", but ArrayBoundsV2 implements complex logic to compare the index with the whole memory region allocated for the array). I'm leaning towards switching to "normal" behavior here.
- Report situations like `struct foo {int field;} arr[5]; arr[10].field = 123;` -- the current implementation doesn't "see" these because this is writing a FieldRegion (which happens to be inside an ElementRegion, but the checker doesn't look for that that).

I'm also open to feedback/suggestions connected to these plans. It's likely that the code added by the current NFC commit will be rewritten by the followup changes, but I think it's still useful to merge it, because it documents the switch from the old implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149259