xazax.hun added a comment. Overall looks good to me. I am curious what will the strategy be to properly support construction. Do you plan to introduce a customization point to `Env.createValue` to give checks/models a way to set properties up? Or do you have something else in mind?
================ Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:219 +/// its location. Returns nullptr if the value can't be represented. +StorageLocation *initializeUnwrappedValue(const Expr &Expr, + StructValue &OptionalVal, ---------------- I recall some problems with convergence due to creating fresh locations lazily in multiple branches. I wonder if doubling down before finding a proper solution could make it harder to fix this in the near future. If this is just a temporary solution until construction is better supported, it might be fine. ================ Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:286 // Record that this unwrap is *not* provably safe. State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc()); } ---------------- I wonder how well this strategy works in practice. If we have many `unwrap` calls to the same `optional`, we might end up emitting lots of diagnostics. An alternative approach is to assume that the `optional` has a value after the first unwrap. This would ensure that we only report a particular problem once and can reduce the noise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124932/new/ https://reviews.llvm.org/D124932 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
