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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
