lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Keep yaffut from attempting to demangle. (issue 5375051)


From: dak
Subject: Re: Keep yaffut from attempting to demangle. (issue 5375051)
Date: Fri, 11 Nov 2011 08:31:05 +0000

Reviewers: J_lowe, jan.nieuwenhuizen,

Message:
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

Description:
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.

Please review this at http://codereview.appspot.com/5375051/

Affected files:
  M flower/include/yaffut.hh


Index: flower/include/yaffut.hh
diff --git a/flower/include/yaffut.hh b/flower/include/yaffut.hh
index 376c036ed1ff617304395f61ac86aca52fe2acf6..5275f487cbaa53f7922b625e1395a2855a062cfd 100644
--- a/flower/include/yaffut.hh
+++ b/flower/include/yaffut.hh
@@ -2,12 +2,12 @@
 // Distributed under the Boost Software License, Version 1.0. (See
 // accompanying file LICENSE_1_0.txt or copy at
 // http://www.boost.org/LICENSE_1_0.txt)
+// Modified to omit demangling, filter through c++filt -t instead

 #ifndef __YAFFUT_H__
 #define __YAFFUT_H__

-#include <cxxabi.h>
-
+#include <typeinfo>
 #include <cmath>
 #include <cstring>
 #include <iostream>
@@ -65,17 +65,7 @@ namespace yaffut
 template <typename T>
 std::string demangle ()
 {
-  size_t sz;
-  int status;
-  char *ptr = abi::__cxa_demangle (typeid (T).name (), 0, &sz, &status);
-  std::string name (ptr ? ptr : "", ptr ? strlen (ptr) : 0);
-  if (ptr) { free (ptr); }
-  std::string::size_type pos = name.rfind ("::");
-  if (pos != std::string::npos)
-    {
-      name = name.substr (pos + 2);
-    }
-  return name;
+  return typeid(T).name ();
 }

 struct ITest





reply via email to

[Prev in Thread] Current Thread [Next in Thread]