Keep yaffut from attempting to demangle.

Keep yaffut from attempting to demangle.

Issue 1875

One can get the pretty output by piping the results through "c++filt
-t", but alas, the stepmake stuff is so totally inscrutable that I
have no idea how one would make the test target do that.

But non-demangled output is better than a complete failure.

[pkx166h, 2011-11-10 20:07:34]

Passes make and no reg test diffs

james

[janneke, 2011-11-11 06:48:28]

On 2011/11/10 20:07:34, J_lowe wrote:
> Passes make and no reg test diffs

+// Modified to omit demangling, filter through c++filt -t instead

Please, do not add changelogs to source files; we have Git for that.
Also, what I miss in this description is *why* you made the change,
I can see that you removed demangling, and you list an alternative:
c++filt -t.

Why do you prefer using c++filt over demangling?

Remember that yaffut is a cross platform test suite, are things
like c++filt available for MSVC, for example?

> but alas, the stepmake stuff is so totally inscrutable that I
> have no idea how one would make the test target do that.

You don't have to be able to do everything yourself, but if
this patch is going in, the corresponding make commands should
be part of it.

+  return typeid(T).name ();

There is a space missing before the first parenthesis.

This

 template <typename T>
 std::string demangle ()
 {
+  return typeid(T).name ();
 }

is the resulting code.  Why is this function called demangle,
there is no demangling going on, is there?

[dak, 2011-11-11 08:31:05]

On 2011/11/11 06:48:28, jan.nieuwenhuizen wrote:
> On 2011/11/10 20:07:34, J_lowe wrote:
> > Passes make and no reg test diffs
> 
> +// Modified to omit demangling, filter through c++filt -t instead
> 
> Please, do not add changelogs to source files; we have Git for that.

I was adhering to the GPL requirement to place prominent notices when modifying
a file.  For a file taken from somewhere else, I consider this appropriate
because people might confuse it with unmodified upstream.


> Also, what I miss in this description is *why* you made the change,
> I can see that you removed demangling, and you list an alternative:
> c++filt -t.

Issue 1875.

> Why do you prefer using c++filt over demangling?

I prefer c++filt over a crash at link time.

> Remember that yaffut is a cross platform test suite, are things
> like c++filt available for MSVC, for example?

Huh?  What concern are the platforms yaffut runs on?  Don't tell me that
Lilypond is one of its main distribution points.  If it is, so much the better
that I added a notice about the change.

The point of importance are the platforms _Lilypond_ runs on.  And it is not
like the test suite will fail without demangling: it just has less readable
output.

> > but alas, the stepmake stuff is so totally inscrutable that I
> > have no idea how one would make the test target do that.
> 
> You don't have to be able to do everything yourself, but if
> this patch is going in, the corresponding make commands should
> be part of it.
> 
> +  return typeid(T).name ();
> 
> There is a space missing before the first parenthesis.

Yup.

> This
> 
>  template <typename T>
>  std::string demangle ()
>  {
> +  return typeid(T).name ();
>  }
> 
> is the resulting code.  Why is this function called demangle,
> there is no demangling going on, is there?

Because it was the simplest possible change.  Well, not quite.  There are a few
other sensible options, partly independent:

a) don't use demangle but a neutral name like "typeformat".
b) comment out using #ifdef
c) also replace callers

[reinhold_fam.tuwien.ac.at, 2011-11-11 11:21:07]

On 2011-11-11 09:31, dak@gnu.org wrote:
>> Remember that yaffut is a cross platform test suite, are things
>> like c++filt available for MSVC, for example?
>
> Huh?  What concern are the platforms yaffut runs on?  Don't tell me that
> Lilypond is one of its main distribution points.  If it is, so much the
> better that I added a notice about the change.
>
> The point of importance are the platforms _Lilypond_ runs on.  And it is
> not like the test suite will fail without demangling: it just has less
> readable output.

If it tries to call an external binary that fails, that's a problem, 
because then make check will fail...
Now, I haven't looked at the patch, so I don't know if the test suite 
will still work if c++filt is not available... In the worst case, we 
need to add a configure check for it.a) don't use demangle but a neutral 
name like "typeformat".

Cheers,
Reinhold

-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
  * Financial&  Actuarial Math., Vienna Univ. of Technology, Austria
  * http://www.fam.tuwien.ac.at/, DVR: 0005886
  * LilyPond, Music typesetting, http://www.lilypond.org

[dak, 2011-11-11 11:32:21]

Reinhold Kainhofer <reinhold@fam.tuwien.ac.at> writes:

> On 2011-11-11 09:31, dak@gnu.org wrote:
>>> Remember that yaffut is a cross platform test suite, are things
>>> like c++filt available for MSVC, for example?
>>
>> Huh?  What concern are the platforms yaffut runs on?  Don't tell me that
>> Lilypond is one of its main distribution points.  If it is, so much the
>> better that I added a notice about the change.
>>
>> The point of importance are the platforms _Lilypond_ runs on.  And it is
>> not like the test suite will fail without demangling: it just has less
>> readable output.
>
> If it tries to call an external binary that fails, that's a problem,
> because then make check will fail...

make check fails for me already.

> Now, I haven't looked at the patch, so I don't know if the test suite
> will still work if c++filt is not available...

The test suite does not even try to use c++filt.  I have made configure
_suggest_ using c++filt.

> In the worst case, we need to add a configure check for it.a) don't
> use demangle but a neutral name like "typeformat".

I've kept the name in this latest patch because I am just using this as
a fallback when demangling fails.

I WASTED 4 HOURS OF DEVELOPER TIME diddling with autoconf to keep the
output demangled _if_ it happens to work.  I won't waste any more time
trying to smuggle c++filt into stepmake's arcane rules.  If Jan wants to
have his pet yaffut.hh further pampered, he can do it himself.

The patch in its current state does not affect a working test suite, and
it lets "make check" finish with slightly less readable output when
cxxabi.h breaks g++ 4.6.  So it is _strictly_ an improvement.

Whoever wants to invest more time in this maintenance hog is free to do
so.  But I refuse to further participate in this silliness.  I got
better things to do.  Any further demands for wasting my time on this
will let me abandon the patch for whoever feels more time should be
wasted.

-- 
David Kastrup

[janneke_gnu.org, 2011-11-11 13:42:58]

David Kastrup writes:

> Issue 1875.

Ah, that's the bit I missed.  Just an artifact of our non-integrated
SCM/Bug-tracker/Patch tracker.

> The patch in its current state does not affect a working test suite, and
> it lets "make check" finish with slightly less readable output when
> cxxabi.h breaks g++ 4.6.  So it is _strictly_ an improvement.

Then commit it already!

Jan

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar®  http://AvatarAcademy.nl

[Graham Percival, 2011-11-11 16:46:24]

On Fri, Nov 11, 2011 at 02:42:55PM +0100, Jan Nieuwenhuizen wrote:
> David Kastrup writes:
> 
> > The patch in its current state does not affect a working test suite, and
> > it lets "make check" finish with slightly less readable output when
> > cxxabi.h breaks g++ 4.6.  So it is _strictly_ an improvement.
> 
> Then commit it already!

Classy.

(the below is NOT directed at David, who is an absolutely
fantastic developer)


"one programmer says it's good.  So let's just bypass the review
process and commit it."

Might I remind you that almost everybody wants to have stable
release more often?  And that the biggest cause of not having
stable releases are regressions?  And that the best time to fix
regressions are *before* they happen?  And that code reviews have
been found to be the best single factor for catching bugs?  And
that nobody (apart from maybe you) actually knows what yaffut is
doing or how it works?

David is good.  Really good.  But he's also human.  Maybe he added
an extra semicolon somewhere that makes the code look ok, but blow
up in certain circumstances.  (I've spent at least 20 hours of my
life on semicolon bugs)
Maybe his changes work great on his machine, but they'll fail when
using a different version of gcc.  Maybe he just has some variable
names that are unclear, or maybe a one-line comment would clarify
something... I mean, after staring at his change to yaffut.hh for
a while, I gather that it's constructing a name for a type... but
if I'd seen
  // construct a name for a type of variable
at the top of the demangle() function, I wouldn't have to spend a
minute or two puzzling through stuff like
  std::string name (ptr ? ptr : "", ptr ? strlen (ptr) : 0);

Of course the lack of a comment like that isn't David's fault;
it's the fault of whoever wrote that code in the first place.  But
this would be a good opportunity to clarify that, for future
programmers.


Not all programmers are as good as you and Han-Wen.  But sadly,
lilypond is being maintained by us stupid programmers.
(although we *do* still have some genius programmers as well, so
don't think that I'm insulting all lilypond developers who read
this.  I'm only claiming that *I'm* a stupid programmer, not you)

If lilypond was alpha quality, it wouldn't matter if things
magically broke.  It's alpha, who cares!

If lilypond was a commercial software product, it wouldn't matter
(much) if we had difficult-to-read code.  If the code can only be
read by developers with 5 years of C++ experience, so what?  ok,
those developers might cost twice as much as junior programmers,
but that's just a business decision -- how much time should the
current programmers spend writing clear code, vs. how much do we
want to pay for hiring new developers.

Unfortunately, most people expect (or at least want) lilypond to
be stable, and we have no budget for hiring experienced
developers.


The 48-hour "patch countdown" is designed to give people a chance
to address both problems -- does the code work, and is it
*obvious* how the code works.  Granted, we rarely take advantage
of the review period to actually discuss what the code is doing
and request more comments... but this *does* happen occasionally,
and I definitely think it's worth keeping the *opportunity* for
idiots like me to ask for help in understanding the code.

- Graham