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