lilypond-devel
[Top][All Lists]
Advanced

[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/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]