Issue 5698: int->vsize in various alignment and page-layout methods

https://sourceforge.net/p/testlilyissues/issues/5698/

fixes warnings

[lemzwerg, 2020-01-27 06:33:09]

LGTM, thanks!

https://codereview.appspot.com/563420043/diff/547500043/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/563420043/diff/547500043/lily/grob.cc#newcode492
lily/grob.cc:492: return real_ext;
whitespace

https://codereview.appspot.com/563420043/diff/547500043/lily/include/align-in...
File lily/include/align-interface.hh (right):

https://codereview.appspot.com/563420043/diff/547500043/lily/include/align-in...
lily/include/align-interface.hh:37: get_pure_minimum_translations (Grob *,
std::vector<Grob *> const &,
maybe indent this line by two spaces?

[hanwenn, 2020-01-27 08:26:17]

LGTM

Not for this change, but could we do a global 


  vsize -> size_t

search & replace.  Do we have a reason to keep our own typedef for this?

[dan_faithful.be, 2020-01-27 12:19:53]

On Jan 27, 2020, at 03:26, hanwenn@gmail.com wrote:
> 
> LGTM
> 
> Not for this change, but could we do a global 
> 
>  vsize -> size_t
> 
> search & replace.  Do we have a reason to keep our own typedef for this?

I fully support this.  I wasn't going to bring it up yet because I worried that
it would lead to a lot of discussion and I have bigger fish to fry, but I would
like to eliminate the following from flower because they reduce clarity.

  * real.hh:typedef double Real;
  * std-string.hh:typedef size_t ssize;
  * std-vector.hh:typedef size_t vsize;

I would like to replace the following with standard types (uint8_t etc.).  The
standard types are portable, but these are not.

  * flower-proto.hh:typedef unsigned char Byte;
  * flower-proto.hh:typedef long long I64;
  * flower-proto.hh:typedef unsigned char U8;
  * flower-proto.hh:typedef short I16;
  * flower-proto.hh:typedef unsigned short U16;
  * flower-proto.hh:typedef unsigned U32;
  * flower-proto.hh:typedef int I32;
  * flower-proto.hh:typedef unsigned long long U64;
— 
Dan

[Dan Eble, 2020-01-27 12:40:38]

https://codereview.appspot.com/563420043/diff/547500043/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/563420043/diff/547500043/lily/grob.cc#newcode492
lily/grob.cc:492: return real_ext;
On 2020/01/27 06:33:09, lemzwerg wrote:
> whitespace

Thank you.  Done in my local copy.

https://codereview.appspot.com/563420043/diff/547500043/lily/include/align-in...
File lily/include/align-interface.hh (right):

https://codereview.appspot.com/563420043/diff/547500043/lily/include/align-in...
lily/include/align-interface.hh:37: get_pure_minimum_translations (Grob *,
std::vector<Grob *> const &,
On 2020/01/27 06:33:09, lemzwerg wrote:
> maybe indent this line by two spaces?

That would be inconsistent with the style in other headers (e.g. smobs.hh,
translator.hh).  If this is a readability issue, would adding a blank line above
and below this declaration help?

[hahnjo, 2020-01-27 12:49:32]

On 2020/01/27 12:19:53, dan_faithful.be wrote:
> On Jan 27, 2020, at 03:26, mailto:hanwenn@gmail.com wrote:
> > 
> > LGTM
> > 
> > Not for this change, but could we do a global 
> > 
> >  vsize -> size_t
> > 
> > search & replace.  Do we have a reason to keep our own typedef for this?
> 
> I fully support this.  I wasn't going to bring it up yet because I worried
that
> it would lead to a lot of discussion and I have bigger fish to fry, but I
would
> like to eliminate the following from flower because they reduce clarity.
> 
>   * real.hh:typedef double Real;
>   * std-string.hh:typedef size_t ssize;
>   * std-vector.hh:typedef size_t vsize;
> 
> I would like to replace the following with standard types (uint8_t etc.).  The
> standard types are portable, but these are not.
> 
>   * flower-proto.hh:typedef unsigned char Byte;
>   * flower-proto.hh:typedef long long I64;
>   * flower-proto.hh:typedef unsigned char U8;
>   * flower-proto.hh:typedef short I16;
>   * flower-proto.hh:typedef unsigned short U16;
>   * flower-proto.hh:typedef unsigned U32;
>   * flower-proto.hh:typedef int I32;
>   * flower-proto.hh:typedef unsigned long long U64;
> — 
> Dan
> 

Fully +1 - one step closer to eventually ditch flower completely. I don't see a
reason for keeping a separate library which nobody else uses...

[Dan Eble, 2020-01-27 12:58:28]

On 2020/01/27 12:49:32, hahnjo wrote:
> Fully +1 - one step closer to eventually ditch flower completely. I don't see
a
> reason for keeping a separate library which nobody else uses...

There is some merit to flower in the area of organization and testability.  It's
nice to be able to test things like Interval_t thoroughly (which I'm not
claiming is currently the case) without relying on the rest of LilyPond.

[dak, 2020-01-27 14:33:35]

Dan Eble <dan@faithful.be> writes:

> On Jan 27, 2020, at 03:26, hanwenn@gmail.com wrote:
>> 
>> LGTM
>> 
>> Not for this change, but could we do a global 
>> 
>>  vsize -> size_t
>> 
>> search & replace.  Do we have a reason to keep our own typedef for this?
>
> I fully support this.  I wasn't going to bring it up yet because I
> worried that it would lead to a lot of discussion and I have bigger
> fish to fry, but I would like to eliminate the following from flower
> because they reduce clarity.
>
>   * real.hh:typedef double Real;
>   * std-string.hh:typedef size_t ssize;
>   * std-vector.hh:typedef size_t vsize;

They fix certain types that are used consistently.  If you say they
"reduce clarity", then every typedef reduces clarity.

ssize and vsize may depend on our internal implementations of
std-string.hh and std-vector.hh.  With the current code base, as far as
I know our own types are gone, so size_t seems fine for replacement.

I am decidedly less sure about Real.  Different sizes than just double
might make sense where it is used, say, as the base type of Offset.  I
am not overly sure that we use it consistently, though.

> I would like to replace the following with standard types (uint8_t
> etc.).  The standard types are portable, but these are not.
>
>   * flower-proto.hh:typedef unsigned char Byte;
>   * flower-proto.hh:typedef long long I64;
>   * flower-proto.hh:typedef unsigned char U8;
>   * flower-proto.hh:typedef short I16;
>   * flower-proto.hh:typedef unsigned short U16;
>   * flower-proto.hh:typedef unsigned U32;
>   * flower-proto.hh:typedef int I32;
>   * flower-proto.hh:typedef unsigned long long U64;

I don't really mind here, but "portable" practically means just portable
to GCC and Clang which closely follows GCC, so it's not a problem in
practice but at most in readability, and the names are clear enough.

At any rate, like with running fixcc.py I think the best point of time
would be when the stable branch has seen a formal end of cherry-picking
in order not to complicate it.  The style changes can happen in parallel
on both release branches.  With regard to the somewhat more invasive
type frobbing, I think I'd wait for it until 2.21.0 has already been
out.

-- 
David Kastrup

[dan_faithful.be, 2020-01-27 14:47:54]

On Jan 27, 2020, at 09:33, David Kastrup <dak@gnu.org> wrote:
> 
>> I would like to replace the following with standard types (uint8_t
>> etc.).  The standard types are portable, but these are not.
...
>>  * flower-proto.hh:typedef unsigned U32;
>>  * flower-proto.hh:typedef int I32;
...
> I don't really mind here, but "portable" practically means just portable
> to GCC and Clang which closely follows GCC, so it's not a problem in

I was thinking of differences in data model, e.g. "unsigned" and "int" might
have as few as 16 bits.  See some examples at
https://en.cppreference.com/w/cpp/language/types .
— 
Dan

[dak, 2020-01-27 15:13:42]

Dan Eble <dan@faithful.be> writes:

> On Jan 27, 2020, at 09:33, David Kastrup <dak@gnu.org> wrote:
>> 
>>> I would like to replace the following with standard types (uint8_t
>>> etc.).  The standard types are portable, but these are not.
> ...
>>>  * flower-proto.hh:typedef unsigned U32;
>>>  * flower-proto.hh:typedef int I32;
> ...
>> I don't really mind here, but "portable" practically means just portable
>> to GCC and Clang which closely follows GCC, so it's not a problem in
>
> I was thinking of differences in data model, e.g. "unsigned" and "int"
> might have as few as 16 bits.  See some examples at
> https://en.cppreference.com/w/cpp/language/types .

None relevant to GCC and Clang as far as I can see.  Can you clarify for
which platforms you expect a practical concern?

-- 
David Kastrup

[dan_faithful.be, 2020-01-27 15:21:46]

On Jan 27, 2020, at 10:13, David Kastrup <dak@gnu.org> wrote:
> 
> None relevant to GCC and Clang as far as I can see.  Can you clarify for
> which platforms you expect a practical concern?

No.  That would require a level of effort that is a reason I never mentioned
these things before.
— 
Dan