I have reviewed the changes to smob and listener.
https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh File lily/include/listener.hh (left): https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#oldcode84 lily/include/listener.hh:84: Listener (Listener const &other); So you're OK with the compiler-generated copy constructor? (Just checking.) https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh File lily/include/listener.hh (right): https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#newcode111 lily/include/listener.hh:111: return *Listener::unsmob (a) == *Listener::unsmob (b) If there are no concerns about dereferencing null pointers, a short comment would be comforting. https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#newcode153 lily/include/listener.hh:153: T *t = dynamic_cast<T*> (T::unsmob (target)); This is not a problem with your change, but I notice that using dynamic_cast after unsmob is very common. This looks like a candidate for future simplification something along these lines: template <class T> static inline T* ly_unsmob(SCM s) { return dynamic_cast<T*>(T::unsmob(s)); } invoked as T* t = ly_unsmob<T>(target); https://codereview.appspot.com/235790043/diff/1/lily/include/listener.hh#newcode164 lily/include/listener.hh:164: unsmob (self)->trampoline_ (target, ev); I suppose that here, unsmob (self) will always succeed because guile would not have found this procedure if the type were wrong? https://codereview.appspot.com/235790043/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
