[PATCH] D150775: [clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

Wed May 17 07:38:37 PDT 2023

mboehme added inline comments.

================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:789
-      // of these accessors.
-      .CaseOfCFGStmt<CallExpr>(valueOperatorCall(std::nullopt),
                                [](const CallExpr *E,
----------------
ymandel wrote:
> this function will be left with only one caller. Maybe inline it? For that matter, should the other caller be changed in some parallel way?
> this function will be left with only one caller. Maybe inline it?

The general style in this file seems to be to pull matcher expressions out into helper functions, even if they're only used in one place. FWIW, I'm personally not convinced this is helpful because you _want_ to be able to see exactly what a `CaseOfCFGStmt` is matching on, so inlining makes sense IMO. But I'm not sure I want to be inconsistent with the general style of this file -- it would make more sense to do a general cleanup. WDYT?

> For that matter, should the other caller be changed in some parallel way?

No, that caller (`buildDiagnoseMatchSwitch()`) does not have the same problem because it looks only at the `optional`, not at the value that is contained in it (see also [D150756](https://reviews.llvm.org/D150756), which makes this explicit), and the distinction between `operator*` and `operator->` (for our purposes) is only in the way they return the contained value.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150775