Re: Likely a good frog project for someone with C knowledge
On Tue, Aug 16, 2011 at 11:14:35PM -0300, Han-Wen Nienhuys wrote: On Tue, Aug 16, 2011 at 7:17 PM, David Kastrup d...@gnu.org wrote: So what does relying on undefined behavior buy us apart from the inability to debug type errors? It buys us time to work on more interesting and more valuable improvements. By amazing coincidence, we have a potential Frog (I heard about her about 24 hours ago) for whom this task is absolutely ideal. She has experience with lilypond, scheme, C++, and assorted other languages. She even has a mentor! I'm not suggesting that a major developer take the time to clean up these things, but this seems like a fantastic way for her to get her feet wet with the scheme/macro/C++ stuff -- and my vague impression is that this is one area that contributors find the most confusing. I've heard both scheme people and C++ people complain that they can work on either scheme or C++, but as soon as a bug touches both areas they give up. If she works on this, asks questions about stuff she doesn't understand, and we add the clarifications to the CG, I think this will be a great initial Frog project. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
2011/8/17 David Kastrup d...@gnu.org Bertrand Bordage bordage.bertr...@gmail.com writes: This would be great if Han-Wen decides to keep it like that. Otherwise there is really a lot of work, with many shortcuts to define. to_boolean (scm_is_pair (x)) That one would be wrong since scm_is_pair already returns a C boolean. I was thinking about scm_pair_p ()... This is typically the kind of mistakes that should be solved. Shame on me, I won't do it again. Where did you find SCM_CONSP ? I don't see it in guile's reference. Maybe something to add to the CG ? Bertrand ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
Graham Percival wrote Wednesday, August 17, 2011 7:41 AM On Tue, Aug 16, 2011 at 11:14:35PM -0300, Han-Wen Nienhuys wrote: On Tue, Aug 16, 2011 at 7:17 PM, David Kastrup d...@gnu.org wrote: So what does relying on undefined behavior buy us apart from the inability to debug type errors? It buys us time to work on more interesting and more valuable improvements. If she works on this, asks questions about stuff she doesn't understand, and we add the clarifications to the CG, I think this will be a great initial Frog project. +1 To me, the important first step is to enshrine the correct constructs for common operations in the CG. Looking through the source code to find examples of coding Scheme/C++ interactions is confusing in the extreme. Even if this is unnecessary for correct operation it is definitely worthwhile to establish a LP convention. We review contributions with nitpicking comments on spelling and punctuation. We have policies on indentation. Surely we should establish a policy for coding these operations in a standard (and correct) way. Trevor - No virus found in this message. Checked by AVG - www.avg.com Version: 10.0.1392 / Virus Database: 1520/3838 - Release Date: 08/16/11 ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
Han-Wen Nienhuys hanw...@gmail.com writes: On Tue, Aug 16, 2011 at 7:17 PM, David Kastrup d...@gnu.org wrote: (and I am speaking as a GUILE developer here as well) So what does relying on undefined behavior buy us apart from the inability to debug type errors? It buys us time to work on more interesting and more valuable improvements. More time than you want. And it drives away prospective contributors who try to get a hang of Lilypond and Guile, read the code and the respective docs, and find that they need to understand all the internals of Guile rather than its documented API before being able to understand what code might and might not be in error, and what code might or might not make the grownups laugh at them because it looks way more carefully written than the rest. I mean, look at the bad code I dug up. Pretty early in the list there was: --- a/lily/general-scheme.cc +++ b/lily/general-scheme.cc @@ -381,7 +381,7 @@ LY_DEFINE (ly_stderr_redirect, ly:stderr-redirect, string m = w; string f = ly_scm2string (file_name); FILE *stderrfile; - if (mode != SCM_UNDEFINED scm_string_p (mode)) + if (scm_is_string (mode)) Why would anybody write a condition like if (mode != SCM_UNDEFINED scm_string_p (mode)) if he did not first try just scm_string_p (mode), could not figure out why it didn't work, and then added a check against SCM_UNDEFINED? This line was committed in 2005 by Jan in its original state. Why would Jan add a check against SCM_UNDEFINED before checking for a string? Most likely because he tried it out and was surprised that an undefined value passed for a string, and then fixed this curiosity by explicitly checking for undefined. Speaking as a long-time lilypond developer, it is my experience that the errors you point out are not a problem (except for the SCM = bool conversion). GUILE's API uses data that can be passed into C functions efficiently as parameters. This means that the SCM type must be a machine word, so the genericity suggested by the GUILE docs are a joke. Except that there are insiders and outsiders to the joke. Do you really consider yourself infallible so that you refuse to write code that would be recognizably correct for someone doing a code review? Code reviews are not fun. And exactly because you want to buy yourself time to work on more interesting and more valuable improvements, you should strive to write code that can be reviewed by lesser people. Those that actually read the APIs of Guile. Or in this case, write code that can be reviewed for correctness by the compiler. The compiler is not overly eager to write more interesting and more valuable improvements, but it is a rather thorough and experienced reviewer. If you feel compelled to change large swaths of source code by substituting x == SCM_EOL with scm_is_eq(x, SCM_EOL), then I can't stop you, but to me it just looks like a waste of time. That would be scm_is_null (x). It is not exactly like the code gets less readable by that substitution. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
Bertrand Bordage bordage.bertr...@gmail.com writes: 2011/8/17 David Kastrup d...@gnu.org Bertrand Bordage bordage.bertr...@gmail.com writes: This would be great if Han-Wen decides to keep it like that. Otherwise there is really a lot of work, with many shortcuts to define. to_boolean (scm_is_pair (x)) That one would be wrong since scm_is_pair already returns a C boolean. I was thinking about scm_pair_p ()... This is typically the kind of mistakes that should be solved. Shame on me, I won't do it again. Where did you find SCM_CONSP ? I don't see it in guile's reference. Maybe something to add to the CG ? Guile reference manual for Guile 1.8, Pair data. It is even indexed. What version of the Guile manual are you using? To quote: Guile implements pairs by mapping the CAR and CDR of a pair directly into the two words of the cell. -- Macro: int SCM_CONSP (SCM X) Return non-zero iff X is a Scheme pair object. -- Macro: int SCM_NCONSP (SCM X) The complement of SCM_CONSP. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
David Kastrup d...@gnu.org writes: Bertrand Bordage bordage.bertr...@gmail.com writes: 2011/8/17 David Kastrup d...@gnu.org Bertrand Bordage bordage.bertr...@gmail.com writes: This would be great if Han-Wen decides to keep it like that. Otherwise there is really a lot of work, with many shortcuts to define. to_boolean (scm_is_pair (x)) That one would be wrong since scm_is_pair already returns a C boolean. I was thinking about scm_pair_p ()... This is typically the kind of mistakes that should be solved. Shame on me, I won't do it again. Where did you find SCM_CONSP ? I don't see it in guile's reference. Maybe something to add to the CG ? Guile reference manual for Guile 1.8, Pair data. It is even indexed. What version of the Guile manual are you using? To quote: Guile implements pairs by mapping the CAR and CDR of a pair directly into the two words of the cell. -- Macro: int SCM_CONSP (SCM X) Return non-zero iff X is a Scheme pair object. -- Macro: int SCM_NCONSP (SCM X) The complement of SCM_CONSP. However, I found that as a reasonably dependable heuristic, functions scm_is_* are implemented as macros after all. That is probably why we have scm_is_eq, but not scm_is_equal which needs to be done by something like scm_is_true (scm_equal_p (...)) instead. Or by ly_is_equal (which we have). So here I would just use scm_is_pair and !scm_is_pair. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
On Mi., 17. Aug. 2011 10:19:33 CEST, David Kastrup d...@gnu.org wrote: I mean, look at the bad code I dug up. Pretty early in the list there was: - if (mode != SCM_UNDEFINED scm_string_p (mode)) + if (scm_is_string (mode)) Yes, that's code that should really be fixed. If you feel compelled to change large swaths of source code by substituting x == SCM_EOL with scm_is_eq(x, SCM_EOL), then I can't stop you, but to me it just looks like a waste of time. That would be scm_is_null (x). It is not exactly like the code gets less readable by that substitution. Here, I agree with David, too. If we have someone who wants to work on them and clean up some code, I have no objection. It just probably won't fix a problem, but improves readability and code style. The only proble I see with the -D compile switch is the code of ly_symbol2scm (which is used several times in almost every file), which does a check if (!cached) to see if the SCM cache has been initialized. How should this be correctly implemented? It is not a check for a scheme value, but builds on the guile internals of how a SCM looks when initialized. BTW, it might be useful to compile with -fno-inline to get warnings from inlined functions directly rather than at the spot where they are called. Cheers, Reinhold ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
Reinhold Kainhofer reinh...@kainhofer.com writes: On Mi., 17. Aug. 2011 10:19:33 CEST, David Kastrup d...@gnu.org wrote: I mean, look at the bad code I dug up. Pretty early in the list there was: - if (mode != SCM_UNDEFINED scm_string_p (mode)) + if (scm_is_string (mode)) Yes, that's code that should really be fixed. If you feel compelled to change large swaths of source code by substituting x == SCM_EOL with scm_is_eq(x, SCM_EOL), then I can't stop you, but to me it just looks like a waste of time. That would be scm_is_null (x). It is not exactly like the code gets less readable by that substitution. Here, I agree with David, too. If we have someone who wants to work on them and clean up some code, I have no objection. It just probably won't fix a problem, but improves readability and code style. The only proble I see with the -D compile switch is the code of ly_symbol2scm (which is used several times in almost every file), which does a check if (!cached) to see if the SCM cache has been initialized. How should this be correctly implemented? It is not a check for a scheme value, but builds on the guile internals of how a SCM looks when initialized. I already wrote that. if (!SCM_UNPACK (cached)) We already use SCM_UNPACK to access raw content when creating smobs (and it is both the only and the canonical way to do this). -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
Reinhold Kainhofer reinh...@kainhofer.com writes: On Mi., 17. Aug. 2011 10:19:33 CEST, David Kastrup d...@gnu.org wrote: I mean, look at the bad code I dug up. Pretty early in the list there was: - if (mode != SCM_UNDEFINED scm_string_p (mode)) + if (scm_is_string (mode)) Yes, that's code that should really be fixed. The + is the indicator of a git diff. I already pushed the results of my cursory grep pattern code review. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Prevents nested tuplets from colliding. (issue 4808082)
I haven't gotten around to making the other changes you pointed out yet, but I will either tonight or tomorrow. Cheers, MS Changes are up and ready for review. Cheers, MS http://codereview.appspot.com/4808082/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix for Issue 620. (issue 4814041)
Pushed as ac7aef03ab9d459a6ea6f03d9c127be150871dd4. Cheers, MS http://codereview.appspot.com/4814041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1628. (issue 4876051)
New patchset uploaded w/ minimal changes made (just the engravers). http://codereview.appspot.com/4876051/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
And another frog task
I just profiled the following code: #include stdlib.h #include libguile.h void *test1(void *n) { for (int i=0; i(int)n; i++) { SCM list = SCM_EOL; SCM *tail = list; for (int k=0; k(int)n; k++) { *tail = scm_cons (SCM_BOOL_F, SCM_EOL); tail = SCM_CDRLOC(*tail); } } return 0; } void* test2(void *n) { for (int i=0; i(int)n; i++) { SCM list = SCM_EOL; for (int k=0; k(int)n; k++) { list = scm_cons (SCM_BOOL_F, list); } list = scm_reverse_x (list, SCM_EOL); } return 0; } main(int ac, char**av) { int n = atoi(av[1]); if (n0) scm_with_guile (test1, (void*)-n); else scm_with_guile (test2, (void*)n); return 0; } I profiled both functions in separate runs to make sure the differences were not because of hot/cold cache or heap. The results were that consing the list together from front to end (test1, corresponding with the Lilypond manner of creating lists) were about 20% slower without, 40% slower with optimization in contrast to consing it together in reverse order, then reversing. Variant 1 looks more efficient and is what Lilypond does when it creates linear lists in order. It is, however, considerably less efficient, _and_ it needs additional variables, _and_ it is quite more complex to understand. In addition, the tail pointer is opaque to the conservative garbage collector, so you need to make sure that you do things in the right order so that the tail does not fall victim to the garbage collector. So it would seem like another worthwhile frog task to get rid of SCM_CDRLOC in the source tree when feasible (when the list is being consulted front-to-back while it is in the process of being created, this would not be possible. However, since tail pointers in this manner can't be kept around beyond the function call, chances are that we have very few such cases if at all). It will simplify the code, make it more readable and faster, and decrease the amount of stack scanning that can go wrong. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: And another frog task
On 8/17/11 5:35 AM, David Kastrup d...@gnu.org wrote: So it would seem like another worthwhile frog task to get rid of SCM_CDRLOC in the source tree when feasible (when the list is being consulted front-to-back while it is in the process of being created, this would not be possible. However, since tail pointers in this manner can't be kept around beyond the function call, chances are that we have very few such cases if at all). It will simplify the code, make it more readable and faster, and decrease the amount of stack scanning that can go wrong. And there are only 29 occurrences, so it's a smaller task the the task of rationalizing the guile interface. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
\On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. SCM context_property_lookup_table; SCM grob_property_lookup_table; SCM prob_property_lookup_table; I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: And another frog task
Am Wednesday, 17. August 2011, 13:42:50 schrieb Carl Sorensen: On 8/17/11 5:35 AM, David Kastrup d...@gnu.org wrote: So it would seem like another worthwhile frog task to get rid of SCM_CDRLOC in the source tree when feasible (when the list is being consulted front-to-back while it is in the process of being created, this would not be possible. However, since tail pointers in this manner can't be kept around beyond the function call, chances are that we have very few such cases if at all). It will simplify the code, make it more readable and faster, and decrease the amount of stack scanning that can go wrong. And there are only 29 occurrences, so it's a smaller task the the task of rationalizing the guile interface. It's smaller, but it requires a deeper analysis of the code (where the list might be used meanwhile during the loop, whether the list might already contain elements,e tc.). Most of the guile interface issues are simply copy-and-paste of very few templates,e.g. -) something == SCM_EOL=scm_is_null (something) -) something == SCM_BOOL_T=scm_is_true (something) -) scm_equal_p (a, b_) == SCM_BOOL_F=!ly_is_equal (a, b) Cheers, Reinhold -- -- Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/ * Financial Actuarial Math., Vienna Univ. of Technology, Austria * http://www.fam.tuwien.ac.at/, DVR: 0005886 * LilyPond, Music typesetting, http://www.lilypond.org ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Check for null pointer
On 8/16/11 11:15 PM, Dan Eble d...@faithful.be wrote: I think the following code should check that ev is not null before dereferencing it. This might be a factor in a segfault I saw. (Sorry to be so vague, but I am not going to build lilypond to test the theory. It would take a credible threat of violence to change my mind.) A grep suggests that there may be other similar cases. void Dispatcher::dispatch (SCM sev) { Stream_event *ev = unsmob_stream_event (sev); SCM class_symbol = ev-get_property (class); Certainly there would be a segfault if sev is null. And perhaps this is the best place to check for it. But this would not be the best place to place a warning and handle the null sev, because there is no context for how the null sev got in the call. Do you have more information about the segfault that you'd be willing to share with us? Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
Am Wednesday, 17. August 2011, 13:53:40 schrieb Carl Sorensen: \On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. SCM context_property_lookup_table; SCM grob_property_lookup_table; SCM prob_property_lookup_table; I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? Note that this is also the assumption on the !cached, even if changed to use SCM_UNPACK: SCM cached; if (!SCM_UNPACK (cached)) ... where SCM_UNPACK is defined in /usr/include/libguile/tags.h to be either #define SCM_UNPACK(x) (x) or #define SCM_UNPACK(x) ((x).n.n) In both cases, the check is whether an uninitialized variable is 0... Cheers, Reinhold -- -- Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/ * Financial Actuarial Math., Vienna Univ. of Technology, Austria * http://www.fam.tuwien.ac.at/, DVR: 0005886 * LilyPond, Music typesetting, http://www.lilypond.org ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
Carl Sorensen c_soren...@byu.edu writes: \On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. SCM context_property_lookup_table; SCM grob_property_lookup_table; SCM prob_property_lookup_table; I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? 0 is not a value that the garbage collector will see meaning in. A detailed analysis will likely find that this causes no problems, but where is the point in keeping code around that requires a detailed analysis? It would be better to spend the analysis on real complications rather than sloppy code. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
On Wed, Aug 17, 2011 at 5:19 AM, David Kastrup d...@gnu.org wrote: --- a/lily/general-scheme.cc +++ b/lily/general-scheme.cc @@ -381,7 +381,7 @@ LY_DEFINE (ly_stderr_redirect, ly:stderr-redirect, string m = w; string f = ly_scm2string (file_name); FILE *stderrfile; - if (mode != SCM_UNDEFINED scm_string_p (mode)) + if (scm_is_string (mode)) Why would anybody write a condition like if (mode != SCM_UNDEFINED scm_string_p (mode)) if he did not first try just scm_string_p (mode), could not figure out why it didn't work, and then added a check against SCM_UNDEFINED? SCM_UNDEFINED is used to indicate a missing optional argument. If you look at general-scheme.cc that should be pretty obvious, because pretty much every function looks like that. I have spent 10 minutes looking in vain through the documentation to find the official solution for optional arguments in the C interface, but haven't been able to find it. In guile there are a bunch of macros in validate.h, but they do not appear in the documentation. Oh, I just found it now. http://www.gnu.org/software/guile/manual/html_node/API-Overview.html#API-Overview For some Scheme functions, some last arguments are optional; the corresponding C function must always be invoked with all optional arguments specified. To get the effect as if an argument has not been specified, pass SCM_UNDEFINED as its value. You can not do this for an argument in the middle; when one argument is SCM_UNDEFINED all the ones following it must be SCM_UNDEFINED as well. I guess we could use SCM_UNBNDP() instead. One thing I dislike about converting to the 'official' API is that it will litter our source-code with equally inscrutable macros. Perhaps we should just roll our own? inline SCM ly_is_unbound(SCM) looks much nicer to me. Speaking as a long-time lilypond developer, it is my experience that the errors you point out are not a problem (except for the SCM = bool conversion). GUILE's API uses data that can be passed into C functions efficiently as parameters. This means that the SCM type must be a machine word, so the genericity suggested by the GUILE docs are a joke. Except that there are insiders and outsiders to the joke. Do you really consider yourself infallible so that you refuse to write code that would be recognizably correct for someone doing a code review? Code reviews are not fun. And exactly because you want to buy yourself time to work on more interesting and more valuable improvements, you should strive to write code that can be reviewed by lesser people. Those that actually read the APIs of Guile. Or in this case, write code that can be reviewed for correctness by the compiler. The compiler is not overly eager to write more interesting and more valuable improvements, but it is a rather thorough and experienced reviewer. If you feel compelled to change large swaths of source code by substituting x == SCM_EOL with scm_is_eq(x, SCM_EOL), then I can't stop you, but to me it just looks like a waste of time. That would be scm_is_null (x). It is not exactly like the code gets less readable by that substitution. it's not the same though. scm_is_null expands to pairs.h:#define scm_is_null(x) (scm_is_null_or_nil(x)) on the plus side, if we use this, we will be the first GNU program to be compatible with the elisp compatibility mode in GUILE that has been almost ready for the last 15 years. -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
On 17/08/11 07:41, Graham Percival wrote: On Tue, Aug 16, 2011 at 11:14:35PM -0300, Han-Wen Nienhuys wrote: On Tue, Aug 16, 2011 at 7:17 PM, David Kastrup d...@gnu.org wrote: So what does relying on undefined behavior buy us apart from the inability to debug type errors? It buys us time to work on more interesting and more valuable improvements. By amazing coincidence, we have a potential Frog (I heard about her about 24 hours ago) for whom this task is absolutely ideal. She has experience with lilypond, scheme, C++, and assorted other languages. She even has a mentor! I'm not suggesting that a major developer take the time to clean up these things, but this seems like a fantastic way for her to get her feet wet with the scheme/macro/C++ stuff -- and my vague impression is that this is one area that contributors find the most confusing. I've heard both scheme people and C++ people complain that they can work on either scheme or C++, but as soon as a bug touches both areas they give up. If she works on this, asks questions about stuff she doesn't understand, and we add the clarifications to the CG, I think this will be a great initial Frog project. Cheers, - Graham 1+ Cheers Ian ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
Han-Wen Nienhuys hanw...@gmail.com writes: On Wed, Aug 17, 2011 at 5:19 AM, David Kastrup d...@gnu.org wrote: That would be scm_is_null (x). It is not exactly like the code gets less readable by that substitution. it's not the same though. scm_is_null expands to pairs.h:#define scm_is_null(x)(scm_is_null_or_nil(x)) That would be a newer development, I guess. In the 1.8 sources this just compares to SCM_EOL. on the plus side, if we use this, we will be the first GNU program to be compatible with the elisp compatibility mode in GUILE that has been almost ready for the last 15 years. I should say that would be rather irrelevant as a design goal. In any case, the respective define in current master (2.whatever) is /* * See the comments preceeding the definitions of SCM_BOOL_F and * SCM_MATCHES_BITS_IN_COMMON in tags.h for more information on * how the following macro works. */ #define scm_is_null_or_nil(x) \ (SCM_MATCHES_BITS_IN_COMMON ((x), SCM_ELISP_NIL, SCM_EOL)) Now that's presumably hardly less efficient, in particular not requiring an additional branch. I have a hard time imagining why it would make sense to let Lisp compatibility invade the API like that, but short of making the assembly code look somewhat unexpected, it would not likely affect performance. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: New engraver for braces (issue 4807053)
That wasn't working 'cause I forgot to add some files to git... This is now fixed. Bertrand http://codereview.appspot.com/4807053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
On Wed, Aug 17, 2011 at 05:53:40AM -0600, Carl Sorensen wrote: \On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? Is there a special rule that compilers will always initalize uninitialized scheme values to zero? Because I discovered a segfault just yesterday (in a different program) that was because of gcc [1] not initalizing a variable to 0. [1] or rather, the C standard does not specify that an uninitalized variable should be set to 0, so I do not blame gcc in the least; it was the programmer at fault. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
- Original Message - From: Graham Percival gra...@percival-music.ca To: Carl Sorensen c_soren...@byu.edu Cc: lilypond-devel Development lilypond-devel@gnu.org Sent: Wednesday, August 17, 2011 5:48 PM Subject: Re: Uninitialized SCM variables On Wed, Aug 17, 2011 at 05:53:40AM -0600, Carl Sorensen wrote: \On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? Is there a special rule that compilers will always initalize uninitialized scheme values to zero? Because I discovered a segfault just yesterday (in a different program) that was because of gcc [1] not initalizing a variable to 0. [1] or rather, the C standard does not specify that an uninitalized variable should be set to 0, so I do not blame gcc in the least; it was the programmer at fault. Cheers, - Graham In C-style languages, uninitialised variable are uninitialised and therefore have an indeterminant value. Hence the danger of uninitialised pointers. Some other languages do initialise them to 0 - visual basic is an example. In more modern languages, (c# is one I'm familiar with) the compile fails if a variable is not explicitly initialised. -- Phil Holmes ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
Graham Percival gra...@percival-music.ca writes: On Wed, Aug 17, 2011 at 05:53:40AM -0600, Carl Sorensen wrote: \On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? Is there a special rule that compilers will always initalize uninitialized scheme values to zero? Because I discovered a segfault just yesterday (in a different program) that was because of gcc [1] not initalizing a variable to 0. The C runtime initializes static storage to binary zeros. That is guaranteed. For automatic variables, all guesses are off. The typical multiuser operating system will initialize stack areas to zeros when they get mapped the first time (uninitialized memory could leak information), but only when the area is used the first time. After that, the values depend on the history of previous function calls. [1] or rather, the C standard does not specify that an uninitalized variable should be set to 0, so I do not blame gcc in the least; it was the programmer at fault. The C standard guarantees binary zeros for statically allocated uninitialized variables. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
Phil Holmes m...@philholmes.net writes: In C-style languages, uninitialised variable are uninitialised and therefore have an indeterminant value. Wrong for statically allocated variables. Hence the danger of uninitialised pointers. Some other languages do initialise them to 0 - visual basic is an example. In more modern languages, (c# is one I'm familiar with) the compile fails if a variable is not explicitly initialised. In C++, the compilation is not guaranteed to succeed if the variable is not explicitly _instantiated_ in some compilation unit (rather than just being declared as extern). An initialization need not happen: binary zeros is the default. Unless we are talking classes with constructors. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
On Wed, Aug 17, 2011 at 07:26:19PM +0200, David Kastrup wrote: Graham Percival gra...@percival-music.ca writes: [1] or rather, the C standard does not specify that an uninitalized variable should be set to 0, so I do not blame gcc in the least; it was the programmer at fault. The C standard guarantees binary zeros for statically allocated uninitialized variables. Ok, in my case it was an uninitalized member variable, on a G5 machine, with something like gcc 4.01 ? It was dying of memory when trying to allocated an array of 96x10497652 doubles, because the programmer forgot to initialize the second variable to 2. I really dislike languages that allow uninitalized variables; I dislike dealing with such problems so much that I'm a fan of the functional bind a value to a variable and never change it approach. (seen in Mozart/Oz, and probably other languages as well) let the compiler optimize for memory reuse and stuff! Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
New short and long lyric ties. (issue 4912041)
Reviewers: , Message: Hi everyone, This follows 8d148ea05fa4b34f8cc3407e112363d715b27ad8 This is fully working, except for a small issue in make doc. The two examples I put in the doc are working alone, but not with make doc: there should be short ties in ~è~, but we mysteriously get medium ties. This works if we change è for an ASCII character. I think this is somehow due to an encoding issue in the make doc process. Cheers, Bertrand Description: New short and long lyric ties. Please review this at http://codereview.appspot.com/4912041/ Affected files: M Documentation/de/notation/vocal.itely M Documentation/es/notation/vocal.itely M Documentation/fr/notation/vocal.itely M Documentation/notation/vocal.itely M mf/feta-ties.mf M scm/define-markup-commands.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: cartouche collides with heading
This looks like a bug. Could you please report it to bug-texinfo (together with a confirmation that the cartouche problem has been solved)? It looks like I'd need to subscribe to another mailing list to do that. Is this true, or can input be made without being subscribed? You can submit bug reports without being subscribed; the list administrator (also Karl Berry) eventually forwards your mail accordingly after some days. If I do need to subscribe, could I pretty please ask you to report it and confirm the other bug fixed? I have a tiny test case that can be submitted, plus details of a revision level that did work and one that doesn't, which I can supply to you ready-to-go. I can also do a bug report if you send me a small test file. Werner ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR Warning added to para for cueduring (issue 4850051)
LGTM. Recently the same issue came up with \quoteDuring http://lists.gnu.org/archive/html/bug-lilypond/2011-08/msg00425.html (that bug report has other issues as well) That is, starting a Staff with a quote prints nothing. \addQuote A {c' d' e' f' c' d' e' f'} \new Staff{ \quoteDuring A s1 r1} Could you add a parallel warning to the \quoteDuring section? http://codereview.appspot.com/4850051/diff/3001/Documentation/notation/staff.itely File Documentation/notation/staff.itely (right): http://codereview.appspot.com/4850051/diff/3001/Documentation/notation/staff.itely#newcode1181 Documentation/notation/staff.itely:1181: Same issue as with \cueDuring, happens with \quoteDuring When a @code{Voice} starts with @code{\quoteDuring} the @code{Voice} context must be explicitly declared. http://codereview.appspot.com/4850051/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixes issue 1628. (issue 4876051)
LGTM Now string numbers move around slurs as well. That didn't work in the old patch, nor in 2.14. The old regtest string-number-around-slur.ly avoided collisions only by accident. Different pitches would cause collisions with the string numbers, but after this patch they really move #'around the slur. http://codereview.appspot.com/4876051/diff/8001/input/regression/string-number-around-slur.ly File input/regression/string-number-around-slur.ly (right): http://codereview.appspot.com/4876051/diff/8001/input/regression/string-number-around-slur.ly#newcode9 input/regression/string-number-around-slur.ly:9: \textLengthOn Alternatively, if you remove \textLengthOn, the numbers go to their original in-in-out positions, and the natural spacing gives a more realistic test. http://codereview.appspot.com/4876051/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
GOP-PROP 9: behavior of make doc (probable decision)
Not much discussion, not much change from the last version. http://lilypond.org/~graham/gop/gop_9.html Proposal summary If there are build problems, then it should be easier to find out why it’s failing. This will be achieved with log files, as well as possibly including scripts which automatically display portions of those log files for a failing build. We will also add targets for building a specific manual (for quick+easy checking of doc work), as well as for building all documentation in a specific language (either English or a translated language). Rationale When the lilypond doc build breaks, it’s too hard to figure out why it broke. We see emails to lilypond-devel approximately once every four months about broken doc builds. On a subjective note, Graham has been the documentation editor since 2003, but even he cannot reliably pinpoint the cause of a failing doc build within 10 minutes. We waste a ridiculous amount of time, effort, and patience on doc build problems. Sea of output Before any of the current work on reducing output from make, the result of a “make doc” was over 500,000 lines of text. The prime reason for the output being so verbose is that all the processes that run as a result of the call to make echo their output to the screen, often in verbose mode. Lilypond itself produces around 370,000 lines of output as a result of lilypond-book building all the snippets. Much of this output can be redirected to logfiles and so the impossible-to-read clutter on the screen is cut out and could be referred to later. Proposal details When you run make doc, * All output will be saved to various log files, with the * exception of output directly from make(1). Note that make(1) refers to a specific executable file on unix computers, and is not a general term for the build system. * By default, no other output will be displayed on the * console, with one exception: if a build fails, we might * display some portion(s) of log file(s) which give useful * clues about the reason for the failure. The user may optionally request additional output to be printed; this is controlled with the VERBOSE=x flag. In such cases, all output will still be written to log files; the console output is strictly additional to the log files. * Logfiles from calling lilypond (as part of lilypond-book) * will go in the relevant * ‘build/out/lybook-db/12/lily-123456.log’ file. All other * logfiles will go in the ‘build/logfiles/’ directory. A single make doc will therefore result in hundreds of log files. Log files produced from individual lilypond runs are not under our control; apart from that, I anticipate having one or two dozen log files. As long as it is clear which log file is associated with which operation(s), I think this is entirely appropriate. The precise implementation will be discussed for specific patches as they appear. * Both stderr and stdout will be saved in *.log. The order of * lines from these streams should be preserved. * There will be no additional “progress messages” during the * build process. If you run make --silent, a non-failing build * should print absolutely nothing to the screen. * Assuming that the loglevels patch is accepted, lilypond * (inside lilypond-book) will be run with –loglevel=WARN. * http://codereview.appspot.com/4822055/ * Ideally, a failing build should provide hints about the * reason why it failed, or at least hints about which log * file(s) to examine. If this proposal is accepted, none of these policies will be assumed to apply to any other aspect of the build system. Policies for any other aspect of the build system will be discussed in separate proposals. Don’t cause more build problems However, there is a danger in this approach, that vital error messages can also be lost, thus preventing the cause of the failure of a make being found. We therefore need to be exceptionally careful to move cautiously, include plenty of tests, and give time for people to experiment/find problems in each stage before proceeding to the next stage. This will be done by starting from individual lilypond calls within lilypond-book, and slowly moving to “larger” targets of the build system – after the individual lilypond calls are are producing the appropriate amount of output and this is saved in the right place and we can automatically isolate parts of a failing build, we will work on lilypond-book in general, and only then will we look at the build system itself. Implementation notes There is an existing make variable QUIET_BUILD, which alter the amount of output being displayed (http://lilypond.org/doc/v2.15/Documentation/contributor/useful-make-variables ). We are not planning on keeping this make variable. The standard way for GNU packages to give more output is with a V=x option. Presumably this is done by increasing x? If we support this option, we should still write log
Re: New engraver for braces (issue 4807053)
Most of my comments below resemble each other, but up here I want to suggest that if you go down the road of generalizing the arpeggio grob, the places you'd have to do work are in the rhythmic column engraver and in the several NoteColumn functions that look for an arpeggio. Instead, this would be generalized to LeftSideIndication and maybe a LeftSideIndicationSpanner to coordinate the horizontal deployment. Great work in making your way through a lot of Lily code to get this up and running! I get the sense that you have a good understanding of the code base, which will serve you well for future endeavors. Cheers, MS http://codereview.appspot.com/4807053/diff/18001/lily/brace-engraver.cc File lily/brace-engraver.cc (right): http://codereview.appspot.com/4807053/diff/18001/lily/brace-engraver.cc#newcode1 lily/brace-engraver.cc:1: /* Check out my comment for Span_brace_engraver. Of all the engravers in LilyPond, I think that the new-fingering-engraver is the one that can act as the best model for the creation of new engravers insofar as it handles many different grobs with the same method. I think that this can be rolled into the arpeggio engraver with a few extra functions. http://codereview.appspot.com/4807053/diff/18001/lily/span-brace-engraver.cc File lily/span-brace-engraver.cc (right): http://codereview.appspot.com/4807053/diff/18001/lily/span-brace-engraver.cc#newcode1 lily/span-brace-engraver.cc:1: /* Is there any way to combine this with the span arpeggio engraver? It seems like stop_translation_timestep could be moved to another function (ie process_spanning_grob) and then could handle multiple grobs (span_brace_, span_arpeggio_, etc.). http://codereview.appspot.com/4807053/diff/18001/lily/span-brace-engraver.cc#newcode6 lily/span-brace-engraver.cc:6: I don't really remember having anything to do with this file...was I involved in this? I'm getting old if I can't remember this sorta thing! If you're comfortable adding your e-mail address, that helps people who want to edit the file later. http://codereview.appspot.com/4807053/diff/18001/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/4807053/diff/18001/scm/define-grobs.scm#newcode501 scm/define-grobs.scm:501: Just food for thought: currently, the Script grob encompasses many things (staccati, tenuti, etc.), all of which could be their own grobs, but are common and multiple enough to be grouped under one grob. In general, I would work with the rule of one, two, and many. That is, if you see something being one grob, then leave it as one. If there are two (or max(ish) three) that are similar (like the piano pedal stuff) it is ok to have multiple grobs. But once you cross the threshold into many, it's better to handle them like scripts are handled. I agree that braces are currently too-coupled with arpeggio's functionality, but before they become a separate grob, I'd encourage you to think about how plausible it is that other grobs like this one will need to be created. If you can envision many, then roll this into arpeggio (or even change the name of arpeggio to SideSpanner to be more neutral about the actual content), allow for multiple ones attached to one note column, have some sorta interface/uber-grob for determining horizontal order, and go down that route. http://codereview.appspot.com/4807053/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: GOP-PROP 8: issue priorities (probable 2)
On Tue, Aug 16, 2011 at 11:20:02AM +0100, Ian Hulin wrote: 1. Some nit-picky stuff to make the proposal crystal-clear to skim-readers like me. See comments below embedded in the your original message text. Thanks, all fixed. 2. I'd like to consider two types to use as additional info to the current ones: Type-User-development and Type-Developer-development. Interesting idea, but I don't see type-user-development as being useful. If somebody wannts a certain behavior -- for whatever reason -- then IMO that's a plain enhancement request. I'll leave the door open to discussing this in a future GOP proposal, though. But right now I want to get the remove priorities proposal passed without getting derailed. On 16/08/11 05:51, Graham Percival wrote: ** Shutting up users This is a proposal. It's a bit formal, so tone down your LilyPond lists persona a bit and call it something like **Identifying user priorities But I don't care about user priorities... ok, I've changed this to reminding users about stars, and rewritten it in with a more netural one. We will remind users that they can 'star' issues which are important to them. They may or may not be relevant to developing the project, or may need to be re-worked in terms of a suggested solution. Using the star system will allow contributors to triage user-supplied issues and then tell the bug squad they intend to spend time on it. The contributor then uses the Type-*** label to do this. I used this instead: We can remind users that they can @qq{star} an issue to indicate that they care about it. Since we resolved to treat developers as independent volunteers, there is no expectation that anybody will look at those stars, but if any developer want to organize their work schedule according to the stars, they are welcome to do so. I like the reminder about GOP-PROP 7. (well, not an explicit reminder, but it's still there in the independent volunteers bit) New version uploaded, and the final one will be tomorrow. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Likely a good frog project for someone with C knowledge
On Wed, Aug 17, 2011 at 9:41 AM, David Kastrup d...@gnu.org wrote: on the plus side, if we use this, we will be the first GNU program to be compatible with the elisp compatibility mode in GUILE that has been almost ready for the last 15 years. I should say that would be rather irrelevant as a design goal. In any case, the respective define in current master (2.whatever) is I should probably start using sarcasm tags. -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
DOC: Revise CG 3.4 Commit Access (issue 4898058)
LGTM, one suggestion. http://codereview.appspot.com/4898058/diff/1/Documentation/contributor/source-code.itexi File Documentation/contributor/source-code.itexi (right): http://codereview.appspot.com/4898058/diff/1/Documentation/contributor/source-code.itexi#newcode1574 Documentation/contributor/source-code.itexi:1574: making commits. An alternate method would be to put the same RSA private+public key on every machine. Since this is aimed at serious developers, let's include both methods. If it confuses somebody, they can always ask. http://codereview.appspot.com/4898058/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
PATCH: 48-hour countdown 21:00 MDT 2011-08-19
For 21:00 MDT Friday August 19 Issue 1349 http://code.google.com/p/lilypond/issues/detail?id=1349: Guile 2.0 compat: Scheme macros (repeated due to revisions since the Monday list) Issue 1779 http://code.google.com/p/lilypond/issues/detail?id=1779: accidentaled notes too far from the barline - R Issue 4188051 http://codereview.appspot.com/4188051/: Remove special case in staff-spacing Issue 804 http://code.google.com/p/lilypond/issues/detail?id=804: Code cleaning: checking types-conversion issues and other build warnings R Issue 472 http://code.google.com/p/lilypond/issues/detail?id=472: collision rest + (accidental/notehead) with beaming - R Issue 4860043 http://codereview.appspot.com/4860043/: Better pure height approximations for beamed rests. Issue 1785 http://code.google.com/p/lilypond/issues/detail?id=1785: Compressible space before the first note of a line - R Issue 4188051 http://codereview.appspot.com/4188051/: Remove special case in staff-spacing Issue 509 http://code.google.com/p/lilypond/issues/detail?id=509: collision nested tuplet numbers - R Issue 4808082 http://codereview.appspot.com/4808082/ (repeated after revisions) Issue 1776 http://code.google.com/p/lilypond/issues/detail?id=1776: Doc: NR - Polymetric Notation \compoundMeter isn't documented - R Issue 4837050 http://codereview.appspot.com/4837050/ Cheers, Colin -- The human race has one really effective weapon, and that is laughter. -- Mark Twain ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Uninitialized SCM variables
On 2011-08-17, at 13:03 , Phil Holmes wrote: - Original Message - From: Graham Percival gra...@percival-music.ca To: Carl Sorensen c_soren...@byu.edu Cc: lilypond-devel Development lilypond-devel@gnu.org Sent: Wednesday, August 17, 2011 5:48 PM Subject: Re: Uninitialized SCM variables On Wed, Aug 17, 2011 at 05:53:40AM -0600, Carl Sorensen wrote: \On 8/16/11 10:25 PM, Dan Eble d...@faithful.be wrote: Is there a reason that these variables in lily/profile.cc don't need to be initialized? I don't have experience with guile, but it looks dangerous. I guess the code in this section relies on the fact that the compiler will initialize the unitialized value to zero. Do you believe that is a problem? Is there a special rule that compilers will always initalize uninitialized scheme values to zero? Because I discovered a segfault just yesterday (in a different program) that was because of gcc [1] not initalizing a variable to 0. [1] or rather, the C standard does not specify that an uninitalized variable should be set to 0, so I do not blame gcc in the least; it was the programmer at fault. Cheers, - Graham In C-style languages, uninitialised variable are uninitialised and therefore have an indeterminant value. Hence the danger of uninitialised pointers. Some other languages do initialise them to 0 - visual basic is an example. In more modern languages, (c# is one I'm familiar with) the compile fails if a variable is not explicitly initialised. Backing up… I believe the compiler will initialize the bits in the aforementioned variables to zero, but is zero a desirable default for SCM variables in general, and these in particular? It also just sank in that in another thread there was a statement that treating a SCM as a boolean is very wrong. That would include a number of lines in ly_property_lookup_stats and note_property_access that use these variables. -- Dan ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Check for null pointer
Carl Sorensen c_sorensen at byu.edu writes: Do you have more information about the segfault that you'd be willing to share with us? What I have so far is a backtrace: http://lists.gnu.org/archive/html/lilypond-devel/2011-08/msg00494.html and a large amount of input spread across many files, which is why I chose to review the lilypond source first. It may also be of interest that I am generating PartCombineMusic with scheme functions other than the stock part combiner. -- Dan ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel