Reviewers: hahnjo, hanwenn, Dan Eble,
https://codereview.appspot.com/559500043/diff/559510043/lily/grob.cc File lily/grob.cc (right): https://codereview.appspot.com/559500043/diff/559510043/lily/grob.cc#newcode733 lily/grob.cc:733: while (unsmob<Grob> (cause)) On 2020/02/21 00:13:25, Dan Eble wrote: > I appreciate that issuing warnings is not performance-sensitive, and that you > simply transplanted this code, but what do you think about avoiding repeating > the unsmob, to set a good example? Something like ... > > while (const Grob *g = unsmob<Grob> (cause)) > { > cause = g->get_property ("cause"); > } Not going to use const here since it is pointless while we have nothing like const_unsmob, and would not use that order anyway since I consider it misleading (it suggests a declaration of a constant, rather than a pointer to a constant). Other than that, fine. Description: Implement Grob::event_cause, Grob::ultimate_event_cause Those were implemented in Grob_info only previously which does not make a lot of sense, given that they don't access anything particular to Grob_info. Also contains commit: Use Grob::event_cause and Grob::ultimate_event_cause where useful Frankly, this one has been bugging me on and off for years. I just had one compilation error too much today. Please review this at https://codereview.appspot.com/559500043/ Affected files (+42, -37 lines): M lily/accidental-engraver.cc M lily/accidental-placement.cc M lily/grob.cc M lily/grob-info.cc M lily/include/grob.hh M lily/lyric-engraver.cc M lily/percent-repeat-item.cc M lily/slur-engraver.cc M lily/tie-engraver.cc Index: lily/accidental-engraver.cc diff --git a/lily/accidental-engraver.cc b/lily/accidental-engraver.cc index f15981ebb4f3d79f8f16ca073ef1c51935303413..877ec1cc5ba0c21d96fb808ab20d11d19741e06c 100644 --- a/lily/accidental-engraver.cc +++ b/lily/accidental-engraver.cc @@ -374,8 +374,8 @@ Accidental_engraver::stop_translation_timestep () { // Don't mark accidentals as "tied" when the pitch is not // actually the same. This is relevant for enharmonic ties. - Stream_event *le = unsmob<Stream_event> (l->get_property ("cause")); - Stream_event *re = unsmob<Stream_event> (r->get_property ("cause")); + Stream_event *le = l->event_cause (); + Stream_event *re = r->event_cause (); if (le && re && !ly_is_equal (le->get_property ("pitch"), re->get_property ("pitch"))) continue; Index: lily/accidental-placement.cc diff --git a/lily/accidental-placement.cc b/lily/accidental-placement.cc index 7965fcb4b9613399eca830f44a9087aa5709f1ee..e34bfb85293b1c55712d8fd3ac175d2169c2603c 100644 --- a/lily/accidental-placement.cc +++ b/lily/accidental-placement.cc @@ -39,9 +39,8 @@ using std::vector; static Pitch * accidental_pitch (Grob *acc) { - SCM cause = acc->get_parent (Y_AXIS)->get_property ("cause"); + Stream_event *mcause = acc->get_parent (Y_AXIS)->event_cause (); - Stream_event *mcause = unsmob<Stream_event> (cause); if (!mcause) { programming_error ("note head has no event cause"); Index: lily/grob-info.cc diff --git a/lily/grob-info.cc b/lily/grob-info.cc index 70e2ef55b4b966019f8b35e9f437d6637eea6ef0..9b1c265db31ceba86eb00caef323dda4b75319ff 100644 --- a/lily/grob-info.cc +++ b/lily/grob-info.cc @@ -46,8 +46,7 @@ Grob_info::Grob_info () Stream_event * Grob_info::event_cause () const { - SCM cause = grob_->get_property ("cause"); - return unsmob<Stream_event> (cause); + return grob_->event_cause (); } Context * @@ -59,12 +58,7 @@ Grob_info::context () const Stream_event * Grob_info::ultimate_event_cause () const { - SCM cause = grob_->self_scm (); - while (unsmob<Grob> (cause)) - { - cause = unsmob<Grob> (cause)->get_property ("cause"); - } - return unsmob<Stream_event> (cause); + return grob_->ultimate_event_cause (); } bool Index: lily/grob.cc diff --git a/lily/grob.cc b/lily/grob.cc index a17869b00adf97f330d16ee23c887132899c780c..c4646b96b59d7bd6bb277e20438d19eef14bb126 100644 --- a/lily/grob.cc +++ b/lily/grob.cc @@ -716,20 +716,36 @@ Grob::internal_vertical_less (Grob *g1, Grob *g2, bool pure) return false; } +/**************************************************************** + CAUSES +****************************************************************/ +Stream_event * +Grob::event_cause () const +{ + SCM cause = get_property ("cause"); + return unsmob<Stream_event> (cause); +} + +Stream_event * +Grob::ultimate_event_cause () const +{ + SCM cause = self_scm (); + while (unsmob<Grob> (cause)) + { + cause = unsmob<Grob> (cause)->get_property ("cause"); + } + return unsmob<Stream_event> (cause); +} + + + /**************************************************************** MESSAGES ****************************************************************/ void Grob::programming_error (const string &s) const { - SCM cause = self_scm (); - while (Grob *g = unsmob<Grob> (cause)) - cause = g->get_property ("cause"); - - /* ES TODO: cause can't be Music*/ - if (Music *m = unsmob<Music> (cause)) - m->origin ()->programming_error (s); - else if (Stream_event *ev = unsmob<Stream_event> (cause)) + if (Stream_event *ev = ultimate_event_cause ()) ev->origin ()->programming_error (s); else ::programming_error (s); @@ -738,14 +754,7 @@ Grob::programming_error (const string &s) const void Grob::warning (const string &s) const { - SCM cause = self_scm (); - while (Grob *g = unsmob<Grob> (cause)) - cause = g->get_property ("cause"); - - /* ES TODO: cause can't be Music*/ - if (Music *m = unsmob<Music> (cause)) - m->origin ()->warning (s); - else if (Stream_event *ev = unsmob<Stream_event> (cause)) + if (Stream_event *ev = ultimate_event_cause ()) ev->origin ()->warning (s); else ::warning (s); Index: lily/include/grob.hh diff --git a/lily/include/grob.hh b/lily/include/grob.hh index d0195610af9f97634057333b595de0e9445783f4..d01fdf8169912cdf8eda02b21eabfbad4b6f348e 100644 --- a/lily/include/grob.hh +++ b/lily/include/grob.hh @@ -24,6 +24,7 @@ #include "virtual-methods.hh" #include "dimension-cache.hh" #include "grob-interface.hh" +#include "lily-proto.hh" #include <set> @@ -117,6 +118,10 @@ public: void instrumented_set_property (SCM, SCM, char const *, int, char const *); void internal_set_property (SCM sym, SCM val); + /* causes */ + Stream_event *event_cause () const; + Stream_event *ultimate_event_cause () const; + /* messages */ void warning (const std::string&) const; void programming_error (const std::string&) const; Index: lily/lyric-engraver.cc diff --git a/lily/lyric-engraver.cc b/lily/lyric-engraver.cc index 6cc5cff89cc2292a3d0088f109f352691d285bfc..1de70bf1ba6cbebc7b5359ea67b5e51231153d3c 100644 --- a/lily/lyric-engraver.cc +++ b/lily/lyric-engraver.cc @@ -146,7 +146,7 @@ get_current_note_head (Context *voice) // It's a bit irritating that we just have the length and // duration of the Grob. Moment end_from_now = - get_event_length (unsmob<Stream_event> (g->get_property ("cause")), now) + get_event_length (g->event_cause (), now) + now; // We cannot actually include more than a single grace note // using busyGrobs on ungraced lyrics since a grob ending on Index: lily/percent-repeat-item.cc diff --git a/lily/percent-repeat-item.cc b/lily/percent-repeat-item.cc index 177266693f6a8ecc2c9b9c4c6c4c1527b0478810..776a8b62a000d2dee3c3f71f35ef2867db1f2620 100644 --- a/lily/percent-repeat-item.cc +++ b/lily/percent-repeat-item.cc @@ -80,7 +80,7 @@ SCM Percent_repeat_item_interface::beat_slash (SCM grob) { Grob *me = unsmob<Grob> (grob); - Stream_event *cause = unsmob<Stream_event> (me->get_property ("cause")); + Stream_event *cause = me->event_cause (); int count = robust_scm2int (cause->get_property ("slash-count"), 1); Stencil m; Index: lily/slur-engraver.cc diff --git a/lily/slur-engraver.cc b/lily/slur-engraver.cc index 8498ebf2abc2002613f4e9b09c745b6795b7b70d..731abc69f8dfd88c398ce516eb2ce6f21e0a9d3a 100644 --- a/lily/slur-engraver.cc +++ b/lily/slur-engraver.cc @@ -151,8 +151,7 @@ Slur_engraver::acknowledge_note_column (Grob_info info) extract_grob_set (e, "note-heads", heads); for (vsize i = heads.size (); i--;) { - if (Stream_event *ev = - unsmob<Stream_event> (heads[i]->get_property ("cause"))) + if (Stream_event *ev = heads[i]->event_cause ()) for (LEFT_and_RIGHT (d)) { std::pair<Note_slurs::const_iterator, Note_slurs::const_iterator> its @@ -249,7 +248,7 @@ Slur_engraver::can_create_slur (SCM id, vsize old_slurs, vsize *event_idx, Strea if (!updown) return false; - Stream_event *c = unsmob<Stream_event> (slur->get_property ("cause")); + Stream_event *c = slur->event_cause (); if (!c) { Index: lily/tie-engraver.cc diff --git a/lily/tie-engraver.cc b/lily/tie-engraver.cc index f31eb177f3e98115fd7811ee91a4d606049ac71e..f05b866e4c6ba3960892683afaff8254db156197 100644 --- a/lily/tie-engraver.cc +++ b/lily/tie-engraver.cc @@ -157,8 +157,8 @@ Tie_engraver::tie_notehead (Grob *h, bool enharmonic) for (vsize i = 0; i < heads_to_tie_.size (); i++) { Grob *th = heads_to_tie_[i].head_; - Stream_event *right_ev = unsmob<Stream_event> (h->get_property ("cause")); - Stream_event *left_ev = unsmob<Stream_event> (th->get_property ("cause")); + Stream_event *right_ev = h->event_cause (); + Stream_event *left_ev = th->event_cause (); /* maybe should check positions too. @@ -282,8 +282,7 @@ Tie_engraver::process_acknowledged () for (vsize i = 0; i < now_heads_.size (); i++) { Grob *head = now_heads_[i]; - Stream_event *left_ev - = unsmob<Stream_event> (head->get_property ("cause")); + Stream_event *left_ev = head->event_cause (); if (!left_ev) {
