[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Use more const Grobs in the Paper_column interface (issue 346100043

From: nine . fierce . ballads
Subject: Re: Use more const Grobs in the Paper_column interface (issue 346100043 by address@hidden)
Date: Tue, 19 Jun 2018 15:41:09 -0700

Reviewers: dak,
File lily/ (right):
lily/ SCM m = me->get_property ("when");
On 2018/06/19 06:08:56, dak wrote:
How much sense does this make?  get_property on callbacks will execute
sorts of callbacks the first time a property is accessed and store a
result.  So the grob is only "const" because accessing its properties
via a linked list where only the data structure organization rather
than the
data itself happens to affect the formal Grob constness.  Which is
sort of an
implementation detail.

If the effect of the callbacks is limited to lazy initialization, I am
comfortable with get_property() being a const method.

Here's wisdom:


Nothing specific triggered this; I just noticed that the names of these
functions sound like they should not mutate the Grobs.

Please review this at

Affected files (+14, -12 lines):
  M lily/include/paper-column.hh
  M lily/

Index: lily/include/paper-column.hh
diff --git a/lily/include/paper-column.hh b/lily/include/paper-column.hh
index 7aa073ca3596461d0a4840d9eacdf492a12bbf4e..2bfede7774b551aee1af10dc70489b9116d55815 100644
--- a/lily/include/paper-column.hh
+++ b/lily/include/paper-column.hh
@@ -49,14 +49,16 @@ public:
   DECLARE_SCHEME_CALLBACK (before_line_breaking, (SCM));

   static int get_rank (Grob const *);
-  static bool is_musical (Grob *);
-  static Moment when_mom (Grob *);
-  static bool is_used (Grob *);
-  static bool is_breakable (Grob *);
-  static bool is_extraneous_column_from_ligature (Grob *);
+  static bool is_musical (Grob const *);
+  static Moment when_mom (Grob const *);
+  static bool is_used (Grob const *);
+  static bool is_breakable (Grob const *);
+  static bool is_extraneous_column_from_ligature (Grob const *);
+  // TODO: Wouldn't it make sense if these Grobs were const?
   static Real minimum_distance (Grob *l, Grob *r);
+  // TODO: Wouldn't it make sense if this Grob were const?
   static Interval break_align_width (Grob *me, SCM align_sym);
-  static Interval get_interface_extent (Grob *column, SCM iface, Axis a);
+ static Interval get_interface_extent (Grob const *column, SCM iface, Axis a);

 #endif // PAPER_COLUMN_HH
Index: lily/
diff --git a/lily/ b/lily/
index be133d3e180c67871a9e757490d8353f58cc853d..e7e3a68fb91c380a1a6ccd55516a72c8ab19fde7 100644
--- a/lily/
+++ b/lily/
@@ -100,7 +100,7 @@ Paper_column::less_than (Grob *const &a,

-Paper_column::when_mom (Grob *me)
+Paper_column::when_mom (Grob const *me)
   SCM m = me->get_property ("when");
   if (Moment *when = unsmob<Moment> (m))
@@ -109,7 +109,7 @@ Paper_column::when_mom (Grob *me)

-Paper_column::is_musical (Grob *me)
+Paper_column::is_musical (Grob const *me)
   SCM m = me->get_property ("shortest-starter-duration");
   Moment s (0);
@@ -119,7 +119,7 @@ Paper_column::is_musical (Grob *me)

-Paper_column::is_used (Grob *me)
+Paper_column::is_used (Grob const *me)
   extract_grob_set (me, "elements", elts);
   if (elts.size ())
@@ -142,7 +142,7 @@ Paper_column::is_used (Grob *me)

-Paper_column::is_breakable (Grob *me)
+Paper_column::is_breakable (Grob const *me)
   return scm_is_symbol (me->get_property ("line-break-permission"));
@@ -233,7 +233,7 @@ LY_DEFINE (ly_paper_column__break_align_width, "ly:paper-column::break-align-wid
   interface and return their combined extent.
-Paper_column::get_interface_extent (Grob *column, SCM iface, Axis a)
+Paper_column::get_interface_extent (Grob const *column, SCM iface, Axis a)
   Interval extent = Interval (0, 0);
   extract_grob_set (column, "elements", elts);
@@ -424,7 +424,7 @@ Paper_column::before_line_breaking (SCM grob)
    function can be removed.
-Paper_column::is_extraneous_column_from_ligature (Grob *me)
+Paper_column::is_extraneous_column_from_ligature (Grob const *me)
   if (!is_musical (me))
     return false;

reply via email to

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