lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

renewed patches


From: lilypond
Subject: renewed patches
Date: Sun, 10 May 2020 21:53:15 +0200

As a result of the comment from Dan I revoke all previous patches I did sent 
today, and have created a new batch of patches.

Those patches do are smaller, and do not include the to-int() conversions I had 
in my previous patches, because further investigation learned that in all cases 
it would be solved by an other design of the underlaying code.

Jaap de Wolff
> -----Oorspronkelijk bericht-----
> Van: address@hidden <address@hidden>
> Verzonden: Sunday, May 10, 2020 4:58 PM
> Aan: 'Dan Eble' <address@hidden>
> CC: 'address@hidden' <address@hidden>
> Onderwerp: RE: Another patch
> 
> Dan,
> 
> As you already did see, I did not a simple cast, but instead my solution is 
> raising
> an error in those cases where a simple cast would change the value tested.
> However, I will follow your suggestion, and break the patch in parts.
> I also will pay attention to your particular example, as I think this is the 
> only
> case of the now solved warnings where the int in a next function is casted to 
> a
> smaller type.
> 
> Jaap de Wolff
> 
> > -----Oorspronkelijk bericht-----
> > Van: Dan Eble <address@hidden>
> > Verzonden: Sunday, May 10, 2020 4:31 PM
> > Aan: address@hidden
> > CC: address@hidden
> > Onderwerp: Re: Another patch
> >
> > On May 10, 2020, at 08:11, address@hidden wrote:
> > >
> > > I did replace all implicit casts to an int by a inline function,
> > > checking if the value is valid, and then casting  to int.
> > >
> > > Together with my previous patch now all but one compiler warnings
> > > are solved.
> >
> > Jaap,
> >
> > I love the fact that you are taking the initiative to work on these.
> > I haven't reviewed them thoroughly, but my first comment is a note of
> > caution.  A couple of warnings, like the printf one, are just simple
> > oversights; however, many of the warnings that remain in LilyPond are the
> tip of an iceberg.
> >
> > There was a discussion on the developer list about simply casting to
> > silence warnings, but more developers were in favor of leaving the
> > warnings in until someone is motivated to fix them properly.
> >
> > I commend you that your work is not just simply casting, but in at
> > least some cases, it still doesn't appear to be going to the depth
> > these issues deserve.  For example, the number of tracks in a MIDI
> > file is a 16-bit number[1], so clipping to INT_MAX isn't quite right.
> > (I've got an old branch where I've fixed the header generation to use
> > uint16_t, but it's still missing code to warn properly when the number
> > of tracks needs to be clipped.  I'll try to rebase that to save you
> > the work.)
> >
> > I encourage you to sort the simple and obvious fixes (like the printf
> > one) into one commit, and the other cases into their own commits.  It
> > will limit the consequences if we later have to revert a change
> > because of a subtle bug that escaped regression testing.
> >
> > Regards,
> > —
> > Dan
> >
> > [1]
> > http://www.music.mcgill.ca/~ich/classes/mumt306/StandardMIDIfileformat
> > .ht
> > ml#BM2_1

Attachment: 0001-Modify-obvious-incorrect-types.patch.txt
Description: Text document

Attachment: 0002-do-not-use-depcrecated-error-verbose-bison-2013.patch.txt
Description: Text document

Attachment: 0003-Solve-Issue-5963.patch.txt
Description: Text document

Attachment: 0004-Valid-declaration-of-unused-arguments.patch.txt
Description: Text document

Attachment: 0005-ignore-conversion-warning-in-lexer.cc.patch.txt
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]