lilypond-devel
[Top][All Lists]
Advanced

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

Re: lily: fix some type conversion warnings (issue 557190043 by address@


From: hanwenn
Subject: Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden)
Date: Wed, 22 Jan 2020 02:15:03 -0800

Reviewers: dan_faithful.be, lemzwerg, Dan Eble,

Message:
On 2020/01/21 15:21:56, Dan Eble wrote:
>
https://codereview.appspot.com/557190043/diff/581490044/lily/pointer-group-interface.cc
> File lily/pointer-group-interface.cc (right):
> 
>
https://codereview.appspot.com/557190043/diff/581490044/lily/pointer-group-interface.cc#newcode30
> lily/pointer-group-interface.cc:30: return arr ? int (arr->size ()) :
0;
> MHO: Return a vsize.  Somewhere, sometime, someone will probably say,
> 
>     my_things_.size () < pgi.count ()
> 
> and the compiler will warn about comparing int to size_t.
> 
> I don't suggest that you switch to vsize and ferret out all the
consequences at
> this time if you don't want to invest your time in that, but that it's
better to
> leave this line alone and live with the warning until there is time to
take a
> proper look.
> 
>
https://codereview.appspot.com/557190043/diff/581490044/lily/quote-iterator.cc
> File lily/quote-iterator.cc (right):
> 
>
https://codereview.appspot.com/557190043/diff/581490044/lily/quote-iterator.cc#newcode104
> lily/quote-iterator.cc:104: int hi = int (scm_c_vector_length (vec));
> If scm_c_vector_length () is returning size_t, then why don't we work
in terms
> of size_t?  Any place I see a cast, especially a C-style cast, I feel
the urge
> to stop and ask, "Why is this OK?"  So I'd rather not see them where
they can be
> avoided.
> 
> https://codereview.appspot.com/557190043/diff/581490044/lily/stem.cc
> File lily/stem.cc (right):
> 
>
https://codereview.appspot.com/557190043/diff/581490044/lily/stem.cc#newcode94
> lily/stem.cc:94: int len = int (scm_ilength (lst)); // -1 for dotted
lists!
> ditto

Can we go back to basics? 

Why do we need to have the distinction between size_t and int? I know
the standard library returns size_t in some places, but is there any
reason for LilyPond to used unsigned integers anywhere? 

I think the most practical solution is to cast any unsigned to int
directly where we get it out of the external library.


 


Description:
lily: fix some type conversion warnings

Please review this at https://codereview.appspot.com/557190043/

Affected files (+10, -11 lines):
  M lily/break-substitution.cc
  M lily/lookup.cc
  M lily/multi-measure-rest-engraver.cc
  M lily/page-spacing-result.cc
  M lily/pointer-group-interface.cc
  M lily/quote-iterator.cc
  M lily/stem.cc


Index: lily/break-substitution.cc
diff --git a/lily/break-substitution.cc b/lily/break-substitution.cc
index 
901f6b0ceb188bcf470b8333ec825d0761aaf3a6..73379ba395af1e99c02b06858d08a3077255f7e7
 100644
--- a/lily/break-substitution.cc
+++ b/lily/break-substitution.cc
@@ -113,13 +113,12 @@ again:
     }
   else if (scm_is_vector (src))
     {
-      int len = scm_c_vector_length (src);
+      size_t len = scm_c_vector_length (src);
       SCM nv = scm_c_make_vector (len, SCM_UNDEFINED);
-      for (int i = 0; i < len; i++)
+      for (size_t i = 0; i < len; i++)
         {
-          SCM si = scm_from_int (i);
-          scm_vector_set_x (nv, si,
-                            do_break_substitution (scm_vector_ref (src, si)));
+          scm_c_vector_set_x (nv, i,
+                              do_break_substitution (scm_c_vector_ref (src, 
i)));
         }
     }
   else if (scm_is_pair (src))
Index: lily/lookup.cc
diff --git a/lily/lookup.cc b/lily/lookup.cc
index 
c7b8cafcc39dcda4204804e03201755dcce20768..117fe96d609b1f283dc11cbb8b4605aba8035d0e
 100644
--- a/lily/lookup.cc
+++ b/lily/lookup.cc
@@ -311,7 +311,7 @@ Lookup::round_filled_polygon (vector<Offset> const &points,
 
       for (vsize i = 0; i < points.size (); i++)
         {
-          int i0 = i;
+          int i0 = (int) i;
           int i1 = (i + 1) % points.size ();
           int i2 = (i + 2) % points.size ();
           Offset p0 = points[i0];
Index: lily/multi-measure-rest-engraver.cc
diff --git a/lily/multi-measure-rest-engraver.cc 
b/lily/multi-measure-rest-engraver.cc
index 
0e8d694cd5a342a5156b350f2cd8a4fc4ea987df..8901cd4d171c6d1da5a98e1c98cc9f9a5988f5cd
 100644
--- a/lily/multi-measure-rest-engraver.cc
+++ b/lily/multi-measure-rest-engraver.cc
@@ -142,7 +142,7 @@ Multi_measure_rest_engraver::initialize_grobs ()
           Spanner *sp = make_spanner ("MultiMeasureRestScript", e->self_scm 
());
           make_script_from_event (sp, context (),
                                   e->get_property ("articulation-type"),
-                                  i);
+                                  int(i));
           SCM dir = e->get_property ("direction");
           if (is_direction (dir))
             sp->set_property ("direction", dir);
Index: lily/page-spacing-result.cc
diff --git a/lily/page-spacing-result.cc b/lily/page-spacing-result.cc
index 
106c7e882295df3f1e99f0fc7971f888acf65d15..bfb25286912ac8229cba54977d89904d53063dcd
 100644
--- a/lily/page-spacing-result.cc
+++ b/lily/page-spacing-result.cc
@@ -41,7 +41,7 @@ Page_spacing_result::average_force () const
   for (vsize i = 0; i < page_count (); i++)
     average_force += force_[i];
 
-  average_force /= page_count ();
+  average_force /= Real (page_count ());
   return average_force;
 }
 
Index: lily/pointer-group-interface.cc
diff --git a/lily/pointer-group-interface.cc b/lily/pointer-group-interface.cc
index 
266fa25ceb7f888d301e1c615aebb3358f59cdbb..68c0b61872e2949b2d0e9f1f45818ff5f577ff1f
 100644
--- a/lily/pointer-group-interface.cc
+++ b/lily/pointer-group-interface.cc
@@ -27,7 +27,7 @@ int
 Pointer_group_interface::count (Grob *me, SCM sym)
 {
   Grob_array *arr = unsmob<Grob_array> (me->internal_get_object (sym));
-  return arr ? arr->size () : 0;
+  return arr ? int (arr->size ()) : 0;
 }
 
 void
Index: lily/quote-iterator.cc
diff --git a/lily/quote-iterator.cc b/lily/quote-iterator.cc
index 
2056cce77074a5acaf675d657279eba862cc3060..2e4481ea8475ab8098da3b2536667aabfdc7c4aa
 100644
--- a/lily/quote-iterator.cc
+++ b/lily/quote-iterator.cc
@@ -101,7 +101,7 @@ int
 binsearch_scm_vector (SCM vec, SCM key, bool (*is_less) (SCM a, SCM b))
 {
   int lo = 0;
-  int hi = scm_c_vector_length (vec);
+  int hi = int (scm_c_vector_length (vec));
 
   /* binary search */
   do
Index: lily/stem.cc
diff --git a/lily/stem.cc b/lily/stem.cc
index 
2a0f6fef8426ec8896bd1cb7a863fd8110771d9e..2a4e9055218ad5e356660a2d3aea991e03094884
 100644
--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -91,7 +91,7 @@ Stem::get_beaming (Grob *me, Direction d)
 
   SCM lst = index_get_cell (pair, d);
 
-  int len = scm_ilength (lst); // -1 for dotted lists!
+  int len = int (scm_ilength (lst)); // -1 for dotted lists!
   return max (len, 0);
 }
 





reply via email to

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