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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 4 weeks ago by Dan Eble
Modified:
9 months, 2 weeks 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
9 months, 4 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 ...
9 months, 4 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): > ...
9 months, 4 weeks ago (2019-12-05 09:11:44 UTC) #3
lemzwerg
> Last time I had a short look, it seems LilyPond is setting > no ...
9 months, 4 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 ...
9 months, 4 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 ...
9 months, 4 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 ...
9 months, 3 weeks ago (2019-12-10 17:12:31 UTC) #7
Dan Eble
rebase, use -std=c++03
9 months, 3 weeks 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 ...
9 months, 3 weeks 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 ...
9 months, 3 weeks 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 ...
9 months, 3 weeks 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 ...
9 months, 3 weeks 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 ...
9 months, 3 weeks 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 ...
9 months, 3 weeks ago (2019-12-13 11:25:24 UTC) #14
lemzwerg
> Somewhat weirdly, if we are making some standard > definite in the calling options, ...
9 months, 2 weeks ago (2019-12-13 13:36:05 UTC) #15
Dan Eble
9 months, 2 weeks 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