|
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
0001-Modify-obvious-incorrect-types.patch.txt
Description: Text document
0002-do-not-use-depcrecated-error-verbose-bison-2013.patch.txt
Description: Text document
0003-Solve-Issue-5963.patch.txt
Description: Text document
0004-Valid-declaration-of-unused-arguments.patch.txt
Description: Text document
0005-ignore-conversion-warning-in-lexer.cc.patch.txt
Description: Text document
[Prev in Thread] | Current Thread | [Next in Thread] |