[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Provide some more smob functionality and use it (issue 135240043 by
From: |
eble |
Subject: |
Re: Provide some more smob functionality and use it (issue 135240043 by address@hidden) |
Date: |
Mon, 01 Sep 2014 01:54:44 +0000 |
https://codereview.appspot.com/135240043/diff/20001/lily/include/smobs.hh
File lily/include/smobs.hh (right):
https://codereview.appspot.com/135240043/diff/20001/lily/include/smobs.hh#newcode93
lily/include/smobs.hh:93: list = scm_cons (p->unprotect (), list);
I'm ignorant of how Guile collects garbage, but hopefully it wouldn't
preempt this after p is unprotected and before scm_cons appropriates it.
https://codereview.appspot.com/135240043/diff/20001/lily/include/smobs.hh#newcode158
lily/include/smobs.hh:158: // constructor, in constrast, may not be
called at all in classes
contrast
https://codereview.appspot.com/135240043/diff/20001/lily/include/smobs.hh#newcode225
lily/include/smobs.hh:225: return is_smob (s) ? SCM_BOOL_T : SCM_BOOL_F;
Wouldn't something like the following be useful here and elsewhere? (For
example, I found ly_grob_pq_less_p right away, and I wasn't even trying
hard.)
SCM to_SCM_BOOL(bool b)
{
return b ? SCM_BOOL_T : SCM_BOOL_F;
}
https://codereview.appspot.com/135240043/diff/20001/lily/include/smobs.hh#newcode248
lily/include/smobs.hh:248: class Smob: public Smob_base<Super> {
Are the compiler-generated copy constructor and assignment operator
going to do the right thing? If so, a comment to that effect would
assure us that you considered them. If not, they should either be
implemented (if useful) or declared but not defined (if not useful).
https://codereview.appspot.com/135240043/