Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

Dev mailing list
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
C++11, use unique_ptr.
I think we *are* willing to do C++11, aren't we?

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

jonas.hahnfeld
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):


https://codereview.appspot.com/545320043/diff/581320043/flower/include/yaffut.hh#newcode146
> flower/include/yaffut.hh:146: // TODO: Once we are willing to require
C++11, use
> unique_ptr.
> I think we *are* willing to do C++11, aren't we?

Last time I had a short look, it seems LilyPond is setting no -std= at
all. This basically means we get the compiler defaults, possibly
changing with the next compiler release. I think we should actually pass
in something to avoid problems right away (-std=c++11 or =gnu++11 ?).

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

Dev mailing list
In reply to this post by Dev mailing list
> Last time I had a short look, it seems LilyPond is setting
> no -std= at all.  This basically means we get the compiler
> defaults, possibly changing with the next compiler release.
> I think we should actually pass in something to avoid
> problems right away (-std=c++11 or =gnu++11 ?).

Good idea.  In case you have time please try it, please check whether it
works.  You have removed the biggest conformance obstacle already, so I
don't expect big difficulties.

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
On 2019/12/05 09:24:32, lemzwerg wrote:
> > I think we should actually pass in something to avoid
> > problems right away (-std=c++11 or =gnu++11 ?).

> Good idea.  In case you have time please try it, please check whether
it works.
> You have removed the biggest conformance obstacle already, so I don't
expect big
> difficulties.

I need to go back to the list archives to be sure, but my recollection
of the last time
we discussed this was that David asked us to wait until 2.20 was
released before
getting into it.

Also, when we move forward, I think we should go all the way to
requiring C++14,
because it smooths some rough edges of features that were introduced in
C++11.
I expressed this opinion before, but I don't think it generated a
discussion.

I don't think we should use GNU extensions if we can avoid it.


https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
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 David having about
waiting for 2.20 to be released.  The point that GUB needs to
continue to work is a good one to remember.

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

jonas.hahnfeld
In reply to this post by Dev mailing list
On 2019/12/10 19:19:10, Dan Eble wrote:
> rebase, use -std=c++03

Just curious, what would it take to just go to C++11 and unique_ptr? As
far as I know, recent versions of GCC default to newer standards anyway,
so it's actually a step back.

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
On 2019/12/10 21:28:14, hahnjo wrote:
> Just curious, what would it take to just go to C++11 and unique_ptr?

I don't know if the compiler that GUB uses properly supports C++11,
and I'm not yet interested in spending my own time to investigate.

I would fully support "someone" finding out which version of g++
fully supports C++14 without any known issues, upgrading GUB
to use it (if necessary), and then making it a requirement.


https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

jonas.hahnfeld
In reply to this post by Dev mailing list
On 2019/12/10 22:28:46, Dan Eble wrote:
> On 2019/12/10 21:28:14, hahnjo wrote:
> > Just curious, what would it take to just go to C++11 and unique_ptr?

> I don't know if the compiler that GUB uses properly supports C++11,
> and I'm not yet interested in spending my own time to investigate.

GUB builds GCC 4.9.4 which is fine for C++11. I'm not sure what the
cross-compiler for macOS supports (and I really dislike how GUB for
macOS blocks progress in a few important points...)

> I would fully support "someone" finding out which version of g++
> fully supports C++14 without any known issues, upgrading GUB
> to use it (if necessary), and then making it a requirement.

Is there a major benefit from using C++14? Not saying that I wouldn't be
in favor, but if we can get C++11 for free...

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

Dev mailing list
In reply to this post by Dev mailing list
> Is there a major benefit from using C++14? Not saying
> that I wouldn't be in favor, but if we can get C++11
> for free...

C++14 is definitely too new.  For example, TeXLive is going to replace
the poppler library with another library written in C because poppler
requires C++14, which too many systems don't provide.

So please work towards C++11.

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
On 2019/12/11 07:17:30, hahnjo wrote:
> On 2019/12/10 22:28:46, Dan Eble wrote:
> > I don't know if the compiler that GUB uses properly supports C++11,
> > and I'm not yet interested in spending my own time to investigate.

> GUB builds GCC 4.9.4 which is fine for C++11.

That depends on your definition of "fine."  I wouldn't want to have my
time
wasted by something like these, for example:

[c++11] trivial class fails std::is_trivial test
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53036

[constexpr] bogus diagnostic "used in its own initializer"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59937


https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

David Kastrup
In reply to this post by Dev mailing list
On 2019/12/11 13:55:56, Dan Eble wrote:
> On 2019/12/11 07:17:30, hahnjo wrote:
> > On 2019/12/10 22:28:46, Dan Eble wrote:
> > > I don't know if the compiler that GUB uses properly supports
C++11,
> > > and I'm not yet interested in spending my own time to investigate.
> >
> > GUB builds GCC 4.9.4 which is fine for C++11.

> That depends on your definition of "fine."  I wouldn't want to have my
time
> wasted by something like these, for example:

> [c++11] trivial class fails std::is_trivial test
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53036

> [constexpr] bogus diagnostic "used in its own initializer"
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59937

Somewhat weirdly, if we are making some standard definite in the calling
options, I'd somehow be happier with C++11.  C++03 seems a bit backward.
  But there's probably no point in upgrading GCC in GUB before we have
retired PowerPC, right?

https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

Dev mailing list
In reply to this post by Dev mailing list
> 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.


https://codereview.appspot.com/545320043/

Reply | Threaded
Open this post in threaded view
|

Re: Issue 5628: fix warnings compiling flower unit tests (issue 545320043 by nine.fierce.ballads@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
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.


https://codereview.appspot.com/545320043/