sgatev marked an inline comment as done. sgatev added a comment. In D124395#3472974 <https://reviews.llvm.org/D124395#3472974>, @xazax.hun wrote:
> Nice! Did you do some measurements? Does this improve the performance or > decrease the memory consumption? I didn't do any measurements :( I implemented this as I'd like to add context-aware join (what we do for booleans, preserving flow condition context) to `UncheckedOptionalAccessModel` and don't really want to make the current approach a pattern. I suspect this could also be useful when modeling other language constructs. ================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:160 + void addFlowConditionConstraint(AtomicBoolValue &Token, + BoolValue &Constraint); + ---------------- xazax.hun wrote: > Could this be const? I think no because we need to pass it to `getOrCreateDisjunctionValue` and `getOrCreateNegationValue`, and their sub-values are not `const` currently. ================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:213 + // + // Flow conditions can depend on other flow conditions (constraints can refer + // to flow condition tokens). The graph of flow condition dependencies is ---------------- xazax.hun wrote: > I think an example would be nice what "depending on" a flow condition means. > A simplistic example with a nested if would probably be the easiest to > understand. > I didn't add an example, but I rephrased this section after adding the `forkFlowCondition` and `joinFlowConditions` methods. Let me know what you think. ================ Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:184-185 + FlowConditionToken = &DACtx->makeFlowConditionToken(); + DACtx->addFlowConditionDependency(*FlowConditionToken, + *Other.FlowConditionToken); + DACtx->addFlowConditionConstraint(*FlowConditionToken, ---------------- xazax.hun wrote: > Do we ever expect to call these functions independently of each other? If no, > maybe `addFlowConditionDependency` should add the constraint as well. I added `forkFlowCondition` and `joinFlowConditions` methods so dependencies are an implementation detail now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124395/new/ https://reviews.llvm.org/D124395 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
