Re: SCM/bool confusion in One_page_breaking::solve (issue 314490043 by address@hidden)

[dak]

Reviewers: pwm,

Message:
On 2017/02/18 18:08:57, pwm wrote:

https://codereview.appspot.com/314490043/diff/1/lily/one-page-breaking.cc
> File lily/one-page-breaking.cc (right):


https://codereview.appspot.com/314490043/diff/1/lily/one-page-breaking.cc#newcode97
> lily/one-page-breaking.cc:97: if (scm_is_true (scm_gr_p (this_pos,
> lowest_line_pos)))
> Thanks for catching this.  I'll try to take a closer look in the next
day or so.

Actually I didn't.  I just put the finishing touches on a task mainly
done by Valentin that made it possible to enable a debugging option in
Guile, and the compiler then dug this one up in consequence as well as a
few others.

Description:
SCM/bool confusion in One_page_breaking::solve

This replaces an accidentally always-true condition with the
originally intended one.

I have no idea about the consequences: it would be best if the
original author Paul Morris (in issue 4752) checked that this now
works as intended and also tried figuring out why we failed to notice
this problem.

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

Affected files (+1, -1 lines):
  M lily/one-page-breaking.cc


Index: lily/one-page-breaking.cc
diff --git a/lily/one-page-breaking.cc b/lily/one-page-breaking.cc
index  
6962aaf20508b1ce93a0e39ba691bb031cccb46a..bba283d9612382eb9a9f5e6de3cd46f15970b9ff  
100644
--- a/lily/one-page-breaking.cc
+++ b/lily/one-page-breaking.cc
@@ -94,7 +94,7 @@ One_page_breaking::solve ()
     {
       SCM this_pos = scm_car (c);
       line_posns.push_back (scm_to_double (this_pos));
-      if (scm_gr_p (this_pos, lowest_line_pos))
+      if (scm_is_true (scm_gr_p (this_pos, lowest_line_pos)))
         lowest_line_pos = this_pos;
     }