================
@@ -636,40 +636,37 @@ class OptionalIntAnalysis final
if (!CS)
return;
const Stmt *S = CS->getStmt();
- auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
- auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
-
- SmallVector<BoundNodes, 1> Matches = match(
- stmt(anyOf(cxxConstructExpr(HasOptionalIntType).bind("construct"),
- cxxOperatorCallExpr(
- callee(cxxMethodDecl(ofClass(OptionalIntRecordDecl))))
- .bind("operator"))),
- *S, getASTContext());
- if (const auto *E = selectFirst<CXXConstructExpr>(
- "construct", Matches)) {
- cast<RecordValue>(Env.getValue(*E))
- ->setProperty("has_value", Env.getBoolLiteralValue(false));
- } else if (const auto *E =
- selectFirst<CXXOperatorCallExpr>("operator", Matches)) {
- assert(E->getNumArgs() > 0);
- auto *Object = E->getArg(0);
- assert(Object != nullptr);
-
- refreshRecordValue(*Object, Env)
- .setProperty("has_value", Env.getBoolLiteralValue(true));
+ const Expr *E = dyn_cast<Expr>(S);
+ if (!E)
+ return;
+
+ if (!E->getType()->isPointerType())
+ return;
+
+ // Make sure we have a `PointerValue` for `E`.
+ auto *PtrVal = cast_or_null<PointerValue>(Env.getValue(*E));
----------------
martinboehme wrote:
Agree that divergence is a concern -- but I do think we need to let checks
create their own values when the framework doesn't. Here's an
[example](https://github.com/google/crubit/blob/d63080a7335ae2babb2f6c48898dc8a680a7d017/nullability/pointer_nullability_analysis.cc#L1355)
of where we do this in Crubit's nullability check (i.e. in production code) to
ensure that every expression of pointer type is associated with a value.
This is needed because the framework itself doesn't create `Value`s for every
expression. As just one example, the framework doesn't create values for
`CallExpr`s, but we definitely need values for these in the nullability check
as a function returning a pointer can have nullability annotations on that
pointer, and we then need to propagate this pointer value, along with its
nullability information.
We could change this by making sure that the framework creates `Value`s for
every expression, but this would come at a cost: We would be creating a lot of
values that any given analysis doesn't need, and this would negatively impact
runtime, memory consumption, and potentially convergence.
AIUI from @gribozavr, the framework's philosophy so far has been to provide
base functionality that is likely to be needed by most or all analyses, and it
lets a specific analysis customize or extend the behavior where the existing
behavior is not sufficient. But this does come with a tradeoff; as you point
out, a badly written analysis can trigger divergence and other unwanted
behavior. IOW, we're relying on the authors of the analysis to know what
they're doing.
@gribozavr Do you have additional thoughts you want to add here?
(For the purposes of this patch, I don't think there's anything actionable
here, so I will go ahead and submit.)
https://github.com/llvm/llvm-project/pull/76042
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
