xazax.hun added inline comments.
================
Comment at:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:81
+auto isOptionalValueOrConversionAssignment() {
+ return cxxOperatorCallExpr(
----------------
While really like the convenience matchers will give us building data flow
analyses, I wonder whether a lot of duplicated work is happening in the
background. E.g. will we end up string comparing the class name of the parent
of the method in the matcher built by `optionalClass` whenever we process an
overloaded `operator=`?
In a handwritten implementation we would only do this once, store the address
of the canonical declaration somewhere and subsequent checks would only look up
the canonical declaration of the called method and do a pointer comparison.
In case this matcher approach is not that efficient, I wonder if it would be
possible to come up with some APIs where the matching is only done once and
subsequent transfer functions would only use the cached pointers to
declarations/types. In case the matchers are already doing something smart in
the background feel free to ignore this comment.
================
Comment at:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:159-160
LatticeTransferState &State) {
- if (auto *OptionalVal = cast_or_null<StructValue>(
- State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer))) {
+ if (auto *OptionalVal = cast_or_null<StructValue>(State.Env.getValue(
+ *ObjectExpr->IgnoreParens(), SkipPast::ReferenceThenPointer))) {
auto *HasValueVal = getHasValue(OptionalVal);
----------------
If we never assign values to `Paren`s, shouldn't `getValue` do the skipping
instead?
================
Comment at:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:242
+
+ const int TemplateParamOptionalWrappersCount = countOptionalWrappers(
+ *MatchRes.Context, stripReference(E->getDirectCallee()
----------------
Is this duplicated across here and `transferValueOrConversionConstructor`?
Should we have a separate function like `IsValueOperation`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121863/new/
https://reviews.llvm.org/D121863
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
