This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH] New C++ abstraction patch
- To: Michael Elizabeth Chastain <chastain at cygnus dot com>
- Subject: Re: [PATCH] New C++ abstraction patch
- From: Daniel Berlin <dberlin at redhat dot com>
- Date: 20 Feb 2001 22:41:53 -0500
- Cc: gdb-patches at sources dot redhat dot com
- References: <200102210135.RAA21426@bosch.cygnus.com>
Michael Elizabeth Chastain <chastain@cygnus.com> writes:
> Hi Daniel,
>
> I have some comments on your patch.
>
> Michael
>
> ===
>
> + gnu-v3-abi.o: gnu-v3-abi.c cp-abi.h
> +
> + gnu-v2-abi.o: gnu-v2-abi.c cp-abi.h
> +
>
> gnu-v3-abi.o and gnu-v2-abi.o also depend on
> $(defs_h) gdb_regex.h gdb_string.h.
>
Okey-dokey.
>
>
> + int gnuv2_vtable_prefix_p (const char * name)
> + {
> + return (((name)[0] == '_'
> + && (((name)[1] == 'V' && (name)[2] == 'T')
> + || ((name)[1] == 'v' && (name)[2] == 't'))
> + && is_cplus_marker ((name)[3])) ||
> + ((name)[0] == '_' && (name)[1] == '_'
> + && (name)[2] == 'v' && (name)[3] == 't' && (name)[4] == '_'));
> + }
>
> Would you consider writing this as:
>
> int gnuv2_vtable_prefix_p (const char * name)
> {
> if (strncmp (name, "_VT", 3) == 0 && is_cplus_marker (name[3]))
> return 1;
> if (strncmp (name, "_vt", 3) == 0 && is_cplus_marker (name[3]))
> return 1;
> if (strncmp (name, "__vt_", 5) == 0)
> return 1;
> return 0;
> }
>
> Or if that is not to your taste, something that is more readable than
> the former macro?
Sure, that's fine, I was just cutting and pasting the v2 stuff, since i
was more concerned with the v3 implementation stuff.
>
> (This is a style issue rather than a correctness issue so I would
> accept whatever you decide to write, including the original).
>
Hey, i'm happy to do whatever makes it easier
>
>
> + //struct cp_abi_ops current_cp_abi;
>
> This would be the first use of // comments in gdb. There are some
> instances of '//', but they are already inside comments, or they are
> part of /* ... *//* ... */ constructions.
Whoops, I forgot to take that out (As I said, i was switching back and
forth by hand).
>
>
>
> + /* FIXME: Stop doing the re_comp's everytime. Use the other regex api
> + and compile the patterns ahead of time */
>
> One of the other uses of re_comp/re_exec is in solib_add, which calls
> symbol_file_add in a loop. I think you're in danger of overwriting the
> single global re_comp buffer.
I was unware.
I'll rewrite it.
>
> There are other uses in target-specific solib loaders, like irix and hpux.
>
> I like regular expressions but I think you need regcomp/regexec/regerror
> rather than re_comp/re_exec.
I do too, I was just trying to avoid having to relearn the horrible.
>
>
>
> + int gnuv3_destructor_prefix_p (const char * name)
> + {
> + re_comp ("[^0-9]D[1-3]Ev");
> + return re_exec (name) != 0;
> + }
>
> libiberty/cp-demangle.c says that dtor-names are D0 D1 D2, not D1 D2
> D3.
You are correct.
I wonder what possessed them to use C1,C2,C3, then D0,D1,D2.
>
> What happens if the target program has a method named TOORAD2Evade?
> I think your r.e. would match that mangled name.
>
Yes. There is nothing I can do about this.
There is no unambiguous destructor prefix.
Welcome to "Hey, let's hardcode this stuff in" hell.
I plan on using [^0-9]D[1-3]Ev$
So if you had TOORAD2Ev, you'd match.
I could also demangle the name, and check for the "~", but without my
ternary search tree stuff that caches demangled name, this would be a
*serious* performance hit.
I could also use [^0-9]D[1-3]Ev$, and if it matches, demangle, then
check for certain.
This would be much better, performance wise.
>
>
> + int gnuv3_vtable_prefix_p (const char * name)
> + {
> + return strncmp (name, "_ZTV", 4) != 0;
> + }
>
> This strncmp() != 0 should be strcmp() == 0. (silly strncmp)
Right again.
Let me rewrite the regex stuff.
--Dan