xazax.hun requested changes to this revision.
xazax.hun added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:350
+ QualType Type = getInnerPointerType(C, E->getType()->getAsCXXRecordDecl());
+ std::tie(Val, NewState) = retrieveOrConjureInnerPtrVal(Reg, E, Type, C);
+ return {Val, NewState};
----------------
Nit: couldn't you just ` return retrieveOrConjureInnerPtrVal(Reg, E, Type, C);`
instead of all the ceremony with `std::tie`?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:365
+ std::tie(FirstPtrVal, State) = makeSValFor(FirstExpr, First);
+ std::tie(SecondPtrVal, State) = makeSValFor(SecondExpr, Second);
+ BinaryOperatorKind BOK =
----------------
I think we might end up losing some information here. Imagine both
`makeSValFor` conjuring a new symbol. Only one of the symbols will be stored in
the state since capturing happend once, when you created the lambda. Maybe it
is better to ask for a state in `makeSValFor` as a parameter instead of doing a
lambda capture.
Moreover, `retrieveOrConjureInnerPtrVal` will not even look at the state
captured by `makeSValFor`. It will ask the `CheckerContext` to get the state,
but that state is the state from the beginning of the function that does not
have any of the modifications you made to it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104616/new/
https://reviews.llvm.org/D104616
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
