sgatev added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:39
+ // represent conjunctions, disjunctions, and negations.
+ AtomicBoolValue,
+ BoolConjunctionValue,
----------------
xazax.hun wrote:
> Do we need `Value` in the `Kind` if we are already in the class `Value`?
> Also, do we need prefix `Conjunction` and `Disjunction` with `Bool`? Will
> there be a `Conjunction` with other types?
We don't need the `Value` suffix in `Kind` and we don't expect other
conjunctions, disjunctions, and negations for now so I simplified the names.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:79
+/// Models a boolean conjunction.
+class BoolConjunctionValue : public BoolValue {
+public:
----------------
xazax.hun wrote:
> Do we want to have a separate class for every binary/unary operator?
> Alternatively, we could follow what the AST is already doing. Having
> BinaryOperator and UnaryOperator classes with a kind. I guess the current
> approach is not too bad for booleans, but if we want to support all integer
> operators as well in the future I wonder if that would end up with too much
> boilerplate.
Great point! Right now we have this subdivision only for booleans and, as you
mentioned, it's not too bad. Definitely worth considering the alternative
design going forward. If you don't mind, I suggest starting with the current
setup, with a FIXME to remind us of the other option. I'd be happy to revisit
this after the next couple of patches.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:98
+
+ // Create fresh values for unknown boolean expressions.
+ if (LHSVal == nullptr)
----------------
xazax.hun wrote:
> I wonder if this `GetOrCreateFresh` style operations will be common enough to
> have a helper for this purpose.
That, or we can ensure that each expression gets assigned a value. Added a
FIXME for now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119953/new/
https://reviews.llvm.org/D119953
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
