Re: Issue 5733: Fix various type-conversion warnings (issue 559450053 by nine.fierce.ball...@gmail.com)

2020-02-04 Thread nine . fierce . ballads
Reviewers: lemzwerg,

Message:
On 2020/02/04 10:45:58, lemzwerg wrote:
> lily/accidental-placement.cc:62: scm_from_long  (stagger ?
context_hash : 1));
> Are you actually trying clang-format?  There's one space too much :-)

I was too lazy to deal with Han-Wen's config file as a patch.  Now that
it's in master, I plan to try it.

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

1: int->size_t in make_script_from_event ()
... and similarly in Fingering_engraver::make_script ().

2: Fix scm_from_... () with wrong types

3: Fix type-conversion warning in Scale
Assert that scale size is less than the max int value.

4: int->vsize in Dot_column

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

Affected files (+23, -26 lines):
  M lily/accidental-placement.cc
  M lily/dot-column.cc
  M lily/fingering-engraver.cc
  M lily/grob-array-scheme.cc
  M lily/include/script-interface.hh
  M lily/scale.cc
  M lily/script-engraver.cc
  M lily/tab-note-heads-engraver.cc


Index: lily/accidental-placement.cc
diff --git a/lily/accidental-placement.cc b/lily/accidental-placement.cc
index 
0ac9fe98bcfcab465d19d1ff80b172827842423f..43b6971b9eb90b41189ee25c77b1ce8b8fbb514f
 100644
--- a/lily/accidental-placement.cc
+++ b/lily/accidental-placement.cc
@@ -56,10 +56,10 @@ Accidental_placement::add_accidental (Grob *me, Grob *a, 
bool stagger, long cont
 return;
 
   a->set_parent (me, X_AXIS);
-  long n = p->get_notename ();
 
   SCM accs = me->get_object ("accidental-grobs");
-  SCM key = scm_cons (scm_from_int (n), scm_from_long  (stagger ? context_hash 
: 1));
+  SCM key = scm_cons (scm_from_int (p->get_notename ()),
+  scm_from_long  (stagger ? context_hash : 1));
   // assoc because we're dealing with pairs
   SCM entry = scm_assoc (key, accs);
   if (scm_is_false (entry))
Index: lily/dot-column.cc
diff --git a/lily/dot-column.cc b/lily/dot-column.cc
index 
744062c0edac9abc0a40d76ccae34b4d5628a5bb..66c9c91c2830ded090224fe1a22402be6b541d36
 100644
--- a/lily/dot-column.cc
+++ b/lily/dot-column.cc
@@ -170,11 +170,11 @@ Dot_column::calc_positioning_done (SCM smob)
   for (vsize j = 0; j < parent_stems.size (); j++)
 {
   Interval chord = Stem::head_positions (parent_stems[j]);
-  int total_room = ((int) chord.length () + 2
-+ scm_to_int (chord_dots_limit)) / 2;
-  int total_dots = dots_each_stem[j].size ();
+  vsize total_room = (static_cast (chord.length ()) + 2
++ scm_to_size_t (chord_dots_limit)) / 2;
+  vsize total_dots = dots_each_stem[j].size ();
   // remove excessive dots from the ends of the stem
-  for (int first_dot = 0; total_dots > total_room; total_dots--)
+  for (vsize first_dot = 0; total_dots > total_room; total_dots--)
 if (0 == (total_dots - total_room) % 2)
   dots_each_stem[j][first_dot++]->suicide ();
 else
Index: lily/fingering-engraver.cc
diff --git a/lily/fingering-engraver.cc b/lily/fingering-engraver.cc
index 
d6a49f2dff55f261d028e849a9ef686f59b0516b..f7b9e68d29e7bf03af74fefb34b1ea08081d2fc4
 100644
--- a/lily/fingering-engraver.cc
+++ b/lily/fingering-engraver.cc
@@ -46,7 +46,7 @@ protected:
   void acknowledge_note_column (Grob_info);
 
 private:
-  void make_script (Direction, Stream_event *, int);
+  void make_script (Direction, Stream_event *, size_t);
 };
 
 void
@@ -100,7 +100,7 @@ Fingering_engraver::process_music ()
 }
 
 void
-Fingering_engraver::make_script (Direction d, Stream_event *r, int i)
+Fingering_engraver::make_script (Direction d, Stream_event *r, size_t i)
 {
   Item *fingering = make_item ("Fingering", r->self_scm ());
 
@@ -111,16 +111,12 @@ Fingering_engraver::make_script (Direction d, 
Stream_event *r, int i)
   Side_position_interface::set_axis (fingering, Y_AXIS);
   Self_alignment_interface::set_aligned_on_parent (fingering, X_AXIS);
 
-  // Hmm
-  int priority = 200;
-  SCM s = fingering->get_property ("script-priority");
-  if (scm_is_number (s))
-priority = scm_to_int (s);
-
   /* See script-engraver.cc */
-  priority += i;
-
-  fingering->set_property ("script-priority", scm_from_int (priority));
+  SCM priority = fingering->get_property ("script-priority");
+  if (!scm_is_number (priority))
+priority = scm_from_int (200); // TODO: Explain magic.
+  priority = scm_sum (priority, scm_from_size_t (i));
+  fingering->set_property ("script-priority", priority);
 
   if (d)
 fingering->set_property ("direction", scm_from_int (d));
Index: lily/grob-array-scheme.cc
diff --git a/lily/grob-array-scheme.cc b/lily/grob-array-scheme.cc
index 
23b6a14174af1f61af143a8528f11b999655aecd..3d9148d97c19e18cfb8b91d714a9a21a33815cc6
 100644
--- a/lily/grob-array-scheme.cc
+++ b/lily/grob-array-scheme.cc
@@ -29,7 +29,7 @@ LY_DEFINE (ly_grob_array_length, "ly:grob-array-length",
   LY_ASSERT_SMOB (Grob_array, grob_arr, 1);
 
   Grob_array *me = unsmob (grob_ar

Issue 5733: Fix various type-conversion warnings (issue 559450053 by nine.fierce.ball...@gmail.com)

2020-02-04 Thread lemzwerg--- via Discussions on LilyPond development
LGTM


https://codereview.appspot.com/559450053/diff/547570044/lily/accidental-placement.cc
File lily/accidental-placement.cc (right):

https://codereview.appspot.com/559450053/diff/547570044/lily/accidental-placement.cc#newcode62
lily/accidental-placement.cc:62: scm_from_long  (stagger ? context_hash
: 1));
Are you actually trying clang-format?  There's one space too much :-)

https://codereview.appspot.com/559450053/