search

fixstars / ion-kit

Modernized graph-based data processing framework
MIT License
7 stars 6 forks source link

Fix/builder reference #250

Closed xinyuli1204 closed 4 months ago

xinyuli1204 commented 4 months ago

Builder should pass by reference to function lower otherwise it will destroy when lower() return

Fixstars-iizuka commented 4 months ago

@xinyuli1204 Thanks for clarifying. I have request for changes.

Builder class is designed to hold smart pointer to Builder::Impl. This means Builder object can be treated as a reference counted object. Following this philosophy, it is better to move dispose calls from Buidler::~Builder into Buidler::Impl::~Impl instead of using C++ reference for lower function.

iitaku commented 4 months ago

LGTM!