Bug 16848 - code in /ext/demangle.h appears broken
Summary: code in /ext/demangle.h appears broken
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 3.4.0
: P2 normal
Target Milestone: 3.4.3
Assignee: Benjamin Kosnik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-08-01 02:51 UTC by Ivan Godard
Modified: 2005-07-23 22:49 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Compiler output (-v) (995 bytes, text/plain)
2004-08-01 02:54 UTC, Ivan Godard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Godard 2004-08-01 02:51:52 UTC
In bug #16845 the suggested workaround for OP was to use the demangler located in /ext/demangle.h. In:

#include    <memory>
#include    <typeinfo>
#include    <ext/demangle.h>
using namespace __gnu_cxx;
using namespace demangler;
using namespace std;
typedef demangle<char, allocator<char> > demang;
int main() {
    implementation_details id;
    const char* name = typeid(int).name();
    demang::type(name, id);
    return 0;
    }

we see several problems:

1) The template "demangle" takes two typename arguments. The first appears to be never used.

2) The template "demangle" is commented as constituting the public interface. However, it exports functions which take mandatory (not with default) arguments of type "implementation_details", which type is declared in the local namespace "demangler". There exists no public data of this type. A public interface that requires use of a private type cannot be called public :-)

3) There is no default value for the allocator (second) argument to template "demangle", forcing the user to explicitly supply one. This violates the conventions used throughout the standard library.

4) There is no default value for the "implementation_details" argument to the member functions of "demangle". As a minimum an overload should default to the conventions observed by the compiler RTTI/debugger interface itself, so that strings produced by the demangler can be compared with those appearing in listings and regression files.

5) There is no usage documentation that either I or Google can find. All examples available refer to the demangler in cxxabi.h, which has a "C" interface and fails to parse mangled types (cf. bug #16845).

6) The above code fails to compile in 3.4.0. I struggled for a while trying to find out what was wrong, but frankly the compiler output for this is something only a mother would love. 

Ivan
Comment 1 Ivan Godard 2004-08-01 02:54:20 UTC
Created attachment 6861 [details]
Compiler output (-v)
Comment 2 Paolo Carlini 2004-08-01 18:44:54 UTC
Carlo, could you please have a look?
Comment 3 Carlo Wood 2004-08-01 20:13:25 UTC
1) The template "demangle" takes two typename arguments. The first appears to be
never used.

The change log says:

2004-02-26  Benjamin Kosnik  <bkoz@redhat.com>

        * include/bits/demangle.h: Add type template parameter to all
        templates with just an Allocator template parameter.

I have no idea why he did that - I remember he asked me to add that patch
and I refused it because I didn't understand it.  Then he didn't explain it
to me but applied this patch (and I have not been aware of that until now).

2) The template "demangle" is commented as constituting the public interface.
However, it exports functions which take mandatory (not with default) arguments
of type "implementation_details", which type is declared in the local namespace
"demangler". There exists no public data of this type. A public interface that
requires use of a private type cannot be called public :-)

Yes and no.  You can consider __gnu_cxx::demangler::implementation_details
public interface if you want - but more importantly - you shouldn't really
use those exported functions.  I suppose that you are right that these should
have a default value perhaps - but then again, it doesn't hurt when programmers
that use this templated demangler to implement their own demangler are forced
to be aware of the fact that they have to decide what implementation details
they want to use.

3) There is no default value for the allocator (second) argument to template
"demangle", forcing the user to explicitly supply one. This violates the
conventions used throughout the standard library.

That is deliberate.  This interface is not intended for normal programmers
but for demangler writers: if you need a demangler that uses a non-standard
allocator - THEN you will use this - otherwise you'd just use the
std::__cxa_demangle standard interface... which used to be based on THIS
templated demangler implementation using the a default allocator.  However,
Benjamin Kosnik removed it:

2004-02-26  Benjamin Kosnik  <bkoz@redhat.com>

        PR libstdc++/10246
        * libsupc++/Makefile.am: Use libiberty demangler.
        (c_sources): Add cp-demangle.c.
        * libsupc++/Makefile.in: Regenerate.
        * src/Makefile.am (sources): Remove demangle.cc.
        * src/Makefile.in: Regenerate.
        * include/Makefile.am (bits_headers): Move demangle.h.
        (ext_headers): ...here.
        * include/Makefile.in: Regenerate.
        * include/bits/demangle.h: Move...
        * include/ext/demangle.h: ...here.
        * src/demangle.cc: Remove.

5) There is no usage documentation that either I or Google can find. All
examples available refer to the demangler in cxxabi.h, which has a "C" interface
and fails to parse mangled types (cf. bug #16845).

True - there is lack of documentation.  However, all the documentation
one really needed was in that default __cxa_demangle implementation...
that is unfortunately been removed.

6) The above code fails to compile in 3.4.0. I struggled for a while trying to
find out what was wrong, but frankly the compiler output for this is something
only a mother would love. 

The code I last maintained compiles perfectly and works.
Someone else broke it - probably one of the patches by Benjamin - which,
again, I wasn't even aware of till now.

I don't consider myself maintainer of this code anymore now.
Thanks

Carlo

PS Using the last demangle.h that I maintained; the correct usage for
   your code snippet would have been like this:

#include <memory>
#include <typeinfo>
#include <iostream>
#include <cstring>      // strlen
#include <bits/demangle.h>

int main(void)
{
  const char* name = typeid(int).name();

  __gnu_cxx::demangler::session<std::allocator<char> > demangle_session(name,
std::strlen(name));
  std::string result;
  demangle_session.decode_type(result);

  std::cout << result << std::endl;

  return 0;
}

Changing bits/demangle.h into ext/demangle.h and adding a 'char' template
parameter doesn't help with the distributed version of demangle.h (I tried
3.4.1).  But it does work with my version:

~/c++/demangler>g++-3.4.1 -I. bug16848.cc
~/c++/demangler>a.out
int
Comment 4 Ivan Godard 2004-08-01 21:02:25 UTC
Well I appear to have been misled by:

  // Public interface
  template<typename Tp, typename Allocator>
    struct demangle

According to *former* maintainer a mere user is not supposed to be using this
code at all - it's for people who are writing their own demangler. In that case why aren't they "protected"?


<begin flame>
Begging your pardon, but how about putting something in the library that let's me print what my bloody type is in text I can read? <cxxabi>::demangle dies on built-in types (cf. bug #16845), and ext/demangle.h is broken and "you shouldn't really use those exported functions."
<end flame>

Ivan
Comment 5 Wolfgang Bangerth 2004-08-02 13:26:04 UTC
libstdc++ people: 
I guess you have to make up your mind: Ivan's request for clarification 
and changes to enhance usability are definitely valid. You can't tell 
him in one PR that the one demangler isn't what he wants to use and 
point him to the other one, and then with the other one tell him that 
this is actually not for public use and therefore not optimized for 
usability. That just doesn't go very well with users. 
 
I think his requests to 
- give the allocator a default argument 
- remove references to internals from the public interface 
- allow code to be compiled 
are valid indeed. Please consider alternatives beyond "this is how it is 
intended". 
 
W. 
Comment 6 Paolo Carlini 2004-08-02 13:43:26 UTC
Wolfgang, I agree completely with you: either Benjamin or myself will fix 
ext/demangler as required. Just allow a few days for Benjamin to be back on line
and comment on the issue. Thanks.
Comment 7 CVS Commits 2004-09-02 16:56:33 UTC
Subject: Bug 16848

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	bkoz@gcc.gnu.org	2004-09-02 16:56:29

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/include: Makefile.am Makefile.in 
Removed files:
	libstdc++-v3/include/ext: demangle.h 

Log message:
	2004-09-02  Benjamin Kosnik  <bkoz@redhat.com>
	
	PR libstdc++/16848
	* include/Makefile.am (ext_headers): Remove demangle.h.
	* include/Makefile.in: Regenerate.
	* include/ext/demangle.h: Remove.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2650&r2=1.2651
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/Makefile.am.diff?cvsroot=gcc&r1=1.83&r2=1.84
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/Makefile.in.diff?cvsroot=gcc&r1=1.109&r2=1.110
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/demangle.h.diff?cvsroot=gcc&r1=1.2&r2=NONE

Comment 8 Andrew Pinski 2004-09-23 18:20:27 UTC
Fixed in 4.0.0 by removing demangle.h.
Comment 9 CVS Commits 2004-09-27 17:26:53 UTC
Subject: Bug 16848

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	bkoz@gcc.gnu.org	2004-09-27 17:26:45

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/include: Makefile.am Makefile.in 
Removed files:
	libstdc++-v3/include/ext: demangle.h 

Log message:
	2004-09-27  Benjamin Kosnik  <bkoz@redhat.com>
	
	PR libstdc++/16848
	* include/Makefile.am (ext_headers): Remove demangle.h.
	* include/Makefile.in: Regenerate.
	* include/ext/demangle.h: Remove.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.179&r2=1.2224.2.180
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/Makefile.am.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.73.4.2&r2=1.73.4.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/Makefile.in.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.94.4.2&r2=1.94.4.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/demangle.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.1.2.1&r2=NONE