Re: [PATCH] Don't check PST is NULL in read_symtab

[Joel Brobecker <brobecker at adacore dot com>]

> FWIW, I agree with both of you.  I agree with assertion's
> roles.  But I also agree with Yao that for functions that implement
> a class-like interface and take a "this" pointer, there's no need
> to sprinkle the codebase with "gdb_assert (self != NULL)" checks.
> BUT (!), when reading one of those functions, it's a bit more obvious
> and self-describing that the function takes a "this"-style pointer
> when the parameter is actually called "self", and / or at least is
> the first parameter in the function's signature.  Like:

I agree with everything said above. Just to be extra clear, in
case I appeared to be insisting - I was just thinking about
the assert, not requesting it.

> static void
> dbx_psymtab_to_symtab_1 (struct objfile *objfile, struct partial_symtab *pst)
> {
> 
> static void
> dbx_psymtab_to_symtab_1 (struct partial_symtab *pst, struct objfile *objfile)
> {
> 
> static void
> dbx_psymtab_to_symtab_1 (struct partial_symtab *self, struct objfile *objfile)
> {

I like "self" quite a bit, except that it's a little bit less
descriptive that "pst".  But for a "method", I think that's
acceptable.

> (It'd be even better for grepability/readability if the implementations
> and hook name agreed, like:
> 
> - result->read_symtab = dbx_psymtab_to_symtab;
> + result->read_symtab = dbx_read_symtab;
> 
> or
> 
> - result->read_symtab = dbx_psymtab_to_symtab;
> + result->psymtab_to_symtab = dbx_psymtab_to_symtab;

Agree as well.  I don't mind making these two changes after Yao's
commit goes it (unless Yao wants to take care of it, I don't mean
to cut in).

-- 
Joel