Re: Use more const Grobs in the Paper_column interface (issue 346100043 by address@hidden)

[nine . fierce . ballads]

Reviewers: dak,


https://codereview.appspot.com/346100043/diff/1/lily/paper-column.cc
File lily/paper-column.cc (right):

https://codereview.appspot.com/346100043/diff/1/lily/paper-column.cc#newcode105
lily/paper-column.cc:105: 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
various
> sorts of callbacks the first time a property is accessed and store a
cached
> result.  So the grob is only "const" because accessing its properties
happens
> 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:
https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-const

Description:
https://sourceforge.net/p/testlilyissues/issues/5349/

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 https://codereview.appspot.com/346100043/

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


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/paper-column.cc
diff --git a/lily/paper-column.cc b/lily/paper-column.cc
index  
be133d3e180c67871a9e757490d8353f58cc853d..e7e3a68fb91c380a1a6ccd55516a72c8ab19fde7  
100644
--- a/lily/paper-column.cc
+++ b/lily/paper-column.cc
@@ -100,7 +100,7 @@ Paper_column::less_than (Grob *const &a,
 }

 Moment
-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)
 }

 bool
-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)
 }

 bool
-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)
 }

 bool
-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.
 */
 Interval
-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.
 */
 bool
-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;