Transform: add print_smob to aid debugging

Transform: add print_smob to aid debugging

[lemzwerg, 2020-04-25 17:48:09]

LGTM

[Dan Eble, 2020-04-25 22:32:55]

https://codereview.appspot.com/561680043/diff/577820044/lily/include/transfor...
File lily/include/transform.hh (right):

https://codereview.appspot.com/561680043/diff/577820044/lily/include/transfor...
lily/include/transform.hh:33: int print_smob (SCM p, scm_print_state *) const;
This function looks out of place in a group of static constants.  How about
moving it below with the rest of the functions?

https://codereview.appspot.com/561680043/diff/577820044/lily/transform.cc
File lily/transform.cc (left):

https://codereview.appspot.com/561680043/diff/577820044/lily/transform.cc#old...
lily/transform.cc:41: #include "offset.hh"
The include order was better before.  If transform.hh is the first header that
transform.cc includes, then if transform.hh lacks any includes, it will be
detected.

Standard headers should follow the rest to make it more likely to that missing
includes in other headers will be detected.  (I think I'm less of a sticker than
Jonas on this point.  The possibility of false negatives is high for commonly
used headers, but it's still a good idea.)

[hanwenn, 2020-04-26 08:46:41]

https://codereview.appspot.com/561680043/diff/577820044/lily/include/transfor...
File lily/include/transform.hh (right):

https://codereview.appspot.com/561680043/diff/577820044/lily/include/transfor...
lily/include/transform.hh:33: int print_smob (SCM p, scm_print_state *) const;
On 2020/04/25 22:32:55, Dan Eble wrote:
> This function looks out of place in a group of static constants.  How about
> moving it below with the rest of the functions?

Done.

https://codereview.appspot.com/561680043/diff/577820044/lily/transform.cc
File lily/transform.cc (left):

https://codereview.appspot.com/561680043/diff/577820044/lily/transform.cc#old...
lily/transform.cc:41: #include "offset.hh"
On 2020/04/25 22:32:55, Dan Eble wrote:
> The include order was better before.  If transform.hh is the first header that
> transform.cc includes, then if transform.hh lacks any includes, it will be
> detected.
> 
> Standard headers should follow the rest to make it more likely to that missing
> includes in other headers will be detected.  (I think I'm less of a sticker
than
> Jonas on this point.  The possibility of false negatives is high for commonly
> used headers, but it's still a good idea.)

Done.

[hanwenn, 2020-04-26 08:46:58]

dan

[hahnjo, 2020-04-27 06:49:54]

https://codereview.appspot.com/561680043/diff/557780043/lily/transform.cc
File lily/transform.cc (right):

https://codereview.appspot.com/561680043/diff/557780043/lily/transform.cc#new...
lily/transform.cc:39: #include <string>
As Dan mentioned "transform.hh" should be the first include and IMO system
includes should be last, even after all other lily includes.

[hanwenn, 2020-04-27 08:09:21]

header ordre

[hanwenn, 2020-04-27 08:23:53]

https://codereview.appspot.com/561680043/diff/557780043/lily/transform.cc
File lily/transform.cc (right):

https://codereview.appspot.com/561680043/diff/557780043/lily/transform.cc#new...
lily/transform.cc:39: #include <string>
On 2020/04/27 06:49:54, hahnjo wrote:
> As Dan mentioned "transform.hh" should be the first include and IMO system
> includes should be last, even after all other lily includes.

see https://codereview.appspot.com/577840051/

[hanwenn, 2020-05-02 22:24:28]

commit 0a63bd48a02ccdaf7253b1e8edb83a44a6573293
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Sat Apr 25 13:09:24 2020 +0200

    Transform: add print_smob to aid debugging