Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1438)

Issue 545320043: Issue 5628: fix warnings compiling flower unit tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by Dan Eble
Modified:
2 months, 1 week ago
Reviewers:
dak, lemzwerg, hahnjo
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

https://sourceforge.net/p/testlilyissues/issues/5628/ To build and run just the flower tests: 0. cd flower 1. make clean 2. make 3. make test This consists of two commits, but they are not reflected in the review patch sets. The first commit has the addition of -std=c++03 to the compiler flags and a few consequent changes to eliminate the use of the GCC typeof() extension and to avoid new warnings. When -std=c++03 is specified, the compiler does not warn about auto_ptr, so there is no longer any change to yaffut.h. The second commit has the remaining, previously reviewed changes to fix flower unit-test warnings about comparing signed to unsigned types.

Patch Set 1 #

Total comments: 1

Patch Set 2 : compile as Cplusplus 2003 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -14 lines) Patch
M flower/include/std-vector.hh View 1 1 chunk +0 lines, -2 lines 0 comments Download
M flower/test-interval-set.cc View 3 chunks +9 lines, -8 lines 0 comments Download
M lily/beam.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/include/system.hh View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/system.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M stepmake/stepmake/c++-vars.make View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 16
Dan Eble
2 months, 2 weeks ago (2019-12-04 22:43:12 UTC) #1
lemzwerg
LGTM https://codereview.appspot.com/545320043/diff/581320043/flower/include/yaffut.hh File flower/include/yaffut.hh (right): https://codereview.appspot.com/545320043/diff/581320043/flower/include/yaffut.hh#newcode146 flower/include/yaffut.hh:146: // TODO: Once we are willing to require ...
2 months, 2 weeks ago (2019-12-05 06:49:34 UTC) #2
hahnjo
On 2019/12/05 06:49:34, lemzwerg wrote: > LGTM > > https://codereview.appspot.com/545320043/diff/581320043/flower/include/yaffut.hh > File flower/include/yaffut.hh (right): > ...
2 months, 2 weeks ago (2019-12-05 09:11:44 UTC) #3
lemzwerg
> Last time I had a short look, it seems LilyPond is setting > no ...
2 months, 2 weeks ago (2019-12-05 09:24:32 UTC) #4
Dan Eble
On 2019/12/05 09:24:32, lemzwerg wrote: > > I think we should actually pass in something ...
2 months, 2 weeks ago (2019-12-05 13:55:00 UTC) #5
Dan Eble
I found a discussion about C++11: https://lists.gnu.org/archive/html/lilypond-devel/2018-07/msg00039.html but I don't see the opinion I remembered ...
2 months, 2 weeks ago (2019-12-05 14:26:22 UTC) #6
Dan Eble
I plan to add -std=c++03 to the flags instead of using pragmas in yaffut.h. I'm ...
2 months, 1 week ago (2019-12-10 17:12:31 UTC) #7
Dan Eble
rebase, use -std=c++03
2 months, 1 week ago (2019-12-10 19:19:10 UTC) #8
hahnjo
On 2019/12/10 19:19:10, Dan Eble wrote: > rebase, use -std=c++03 Just curious, what would it ...
2 months, 1 week ago (2019-12-10 21:28:14 UTC) #9
Dan Eble
On 2019/12/10 21:28:14, hahnjo wrote: > Just curious, what would it take to just go ...
2 months, 1 week ago (2019-12-10 22:28:46 UTC) #10
hahnjo
On 2019/12/10 22:28:46, Dan Eble wrote: > On 2019/12/10 21:28:14, hahnjo wrote: > > Just ...
2 months, 1 week ago (2019-12-11 07:17:30 UTC) #11
lemzwerg
> Is there a major benefit from using C++14? Not saying > that I wouldn't ...
2 months, 1 week ago (2019-12-11 08:55:07 UTC) #12
Dan Eble
On 2019/12/11 07:17:30, hahnjo wrote: > On 2019/12/10 22:28:46, Dan Eble wrote: > > I ...
2 months, 1 week ago (2019-12-11 13:55:56 UTC) #13
dak
On 2019/12/11 13:55:56, Dan Eble wrote: > On 2019/12/11 07:17:30, hahnjo wrote: > > On ...
2 months, 1 week ago (2019-12-13 11:25:24 UTC) #14
lemzwerg
> Somewhat weirdly, if we are making some standard > definite in the calling options, ...
2 months, 1 week ago (2019-12-13 13:36:05 UTC) #15
Dan Eble
2 months, 1 week ago (2019-12-13 15:00:11 UTC) #16
On 2019/12/13 13:36:05, lemzwerg wrote:
> > Somewhat weirdly, if we are making some standard
> > definite in the calling options, I'd somehow be
> > happier with C++11.
> 
> +1.  AFAICS, C++11 is a major target for virtually all C++ compilers.

Well, I'm certainly not going to stand in the way of this show of support.
I've been wishing to be able to write
    auto col = unsmob<Paper_column>(s);
and
    SomeClass(const SomeClass&) = delete;
for a while.

For this issue, I will push just the changes to flower/test-interval-set.cc that
fix signed-comparison warnings.  Those already made it through a full countdown
cycle, so I will push them very soon.

Then I will create a new issue to add -std=c++11 to the compiler options.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b