[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);
}
- lily: fix some type conversion warnings (issue 557190043 by address@hidden), lemzwerg, 2020/01/20
- Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden), nine . fierce . ballads, 2020/01/21
- Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden),
hanwenn <=
- Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden), hanwenn, 2020/01/22
- Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden), benko . pal, 2020/01/22
- Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden), nine . fierce . ballads, 2020/01/23
- Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden), hanwenn, 2020/01/23
Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden), nine . fierce . ballads, 2020/01/23