iAI icon indicating copy to clipboard operation
iAI copied to clipboard

Eliminate unnecessary indirection layers FlowFunctionType, FlowFunctionTypePtr, and container_type

Open blipper opened this issue 2 years ago • 2 comments

Bug description

Understanding the Phasar framework is made more complicated than necessary due to the extra FlowFunction and FlowFunctionPtr layers.

Issues with the current setup

  • It isn't obvious that FlowFunctionPtr is actually a shard_ptr which is slightly dangerous

  • It requires multiple definitions and imports for that is a pretty simple template with 1 or two parameters.

    using FlowFunctionType = FlowFunction<D, Container>; using FlowFunctionPtrType = std::shared_ptr<FlowFunctionType>;

Imports in using classes using typename FlowFunction<D, Container>::FlowFunctionType; using typename FlowFunction<D, Container>::FlowFunctionPtrType;

  • The auto keyword can be used for local variables to reduce boilerplate.
  • Return parameter boilerplate can be reduced by only specifying 1 parameters IDESolverTest::FlowFunctionPtrType IDESolverTest::getNormalFlowFunction(IDESolverTest::n_t Curr, IDESolverTest::n_t Succ) {} ->

std::shared_ptr<FlowFunction<d_t>> IDESolverTest::getNormalFlowFunction(IDESolverTest::n_t Curr, IDESolverTest::n_t Succ) {}

Possible solution

  • Eliminate them!

blipper avatar Aug 24 '21 16:08 blipper

These were inserted to make a possible change of the underlying types at some point in the future possible in a consistent way throughout the framework. There have been discussions about that particularly between @vulder and @pdschubert, who, I believe, had thought about the point you are making. @vulder, @pdschubert what is your take on this issue?

MMory avatar May 06 '22 13:05 MMory

Especially, for performance tuning, debugging or testing new things out it's quite handy to have only a few central definitions for these. I personally prefer to have some central type defs.

Regarding the FlowFunctionPtrType, yeah ... at some point I would love to get rid of the share_ptr but our last expedition into this valley did not end well, as we still don't no ALL the places where we need to release the memory. However, having the using decl at least made testing things out easier. Maybe, we'll fine a solution to the share_ptr problem at somepoint.

vulder avatar May 07 '22 12:05 vulder