FYI: use SYMBOL_SYMTAB accessor

FYI: use SYMBOL_SYMTAB accessor

[Tom Tromey]

While working on a different patch I noticed a few spots that were not
using the SYMBOL_SYMTAB accessor macro.

This patch fixes these places.  I'm checking it in as obvious.

Tested by rebuilding.

Tom

2012-11-26  Tom Tromey  <tromey@redhat.com>

	* ada-lang.c (user_select_syms): Use SYMBOL_SYMTAB.
	* dwarf2read.c (dw2_find_symbol_file, fixup_go_packaging): Use
	SYMBOL_SYMTAB.
	* skip.c (skip_info): Use SYMBOL_SYMTAB.

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.380
diff -u -r1.380 ada-lang.c
--- ada-lang.c	12 Nov 2012 17:14:54 -0000	1.380
+++ ada-lang.c	26 Nov 2012 15:51:01 -0000
@@ -3583,7 +3583,7 @@
             (SYMBOL_CLASS (syms[i].sym) == LOC_CONST
              && SYMBOL_TYPE (syms[i].sym) != NULL
              && TYPE_CODE (SYMBOL_TYPE (syms[i].sym)) == TYPE_CODE_ENUM);
-          struct symtab *symtab = syms[i].sym->symtab;
+          struct symtab *symtab = SYMBOL_SYMTAB (syms[i].sym);
 
           if (SYMBOL_LINE (syms[i].sym) != 0 && symtab != NULL)
             printf_unfiltered (_("[%d] %s at %s:%d\n"),
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.713
diff -u -r1.713 dwarf2read.c
--- dwarf2read.c	20 Nov 2012 22:51:04 -0000	1.713
+++ dwarf2read.c	26 Nov 2012 15:51:06 -0000
@@ -3402,7 +3402,7 @@
 	  struct symbol *sym = lookup_block_symbol (block, name, VAR_DOMAIN);
 
 	  if (sym)
-	    return sym->symtab->filename;
+	    return SYMBOL_SYMTAB (sym)->filename;
 	}
       return NULL;
     }
@@ -6754,8 +6754,9 @@
 		  if (strcmp (package_name, this_package_name) != 0)
 		    complaint (&symfile_complaints,
 			       _("Symtab %s has objects from two different Go packages: %s and %s"),
-			       (sym->symtab && sym->symtab->filename
-				? sym->symtab->filename
+			       (SYMBOL_SYMTAB (sym)
+				&& SYMBOL_SYMTAB (sym)->filename
+				? SYMBOL_SYMTAB (sym)->filename
 				: cu->objfile->name),
 			       this_package_name, package_name);
 		  xfree (this_package_name);
Index: skip.c
===================================================================
RCS file: /cvs/src/src/gdb/skip.c,v
retrieving revision 1.8
diff -u -r1.8 skip.c
--- skip.c	9 Aug 2012 06:48:21 -0000	1.8
+++ skip.c	26 Nov 2012 15:51:06 -0000
@@ -304,7 +304,7 @@
 	   if (sym)
 	     ui_out_field_fmt (current_uiout, "what", "%s at %s:%d",
 			       sym->ginfo.name,
-			       sym->symtab->filename,
+			       SYMBOL_SYMTAB (sym)->filename,
 			       sym->line);
 	   else
 	     ui_out_field_string (current_uiout, "what", "?");

Re: FYI: use SYMBOL_SYMTAB accessor

[Joel Brobecker]

> While working on a different patch I noticed a few spots that were not
> using the SYMBOL_SYMTAB accessor macro.
> 
> This patch fixes these places.  I'm checking it in as obvious.

Thanks!

I am wondering if should consider making these structures opaque
Aside from the amount of work to do in order to achieve that,
do you see a negative is aiming for this goal for this particular
struct? (and all the symtab-related structs)?

[I've always wanted to make all complex and semi-complex structures
opaque :-)]

-- 
Joel

Re: FYI: use SYMBOL_SYMTAB accessor

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I am wondering if should consider making these structures opaque
Joel> Aside from the amount of work to do in order to achieve that,
Joel> do you see a negative is aiming for this goal for this particular
Joel> struct? (and all the symtab-related structs)?

Joel> [I've always wanted to make all complex and semi-complex structures
Joel> opaque :-)]

I'm somewhat on the fence about this, but on balance I think it is a
good idea to make types that are used in more than one file opaque.

However, in practice this hasn't always helped much.  The symbol
accessors are the worst, as we've discussed before, because many of them
are polymorphic without any purpose -- so they are both verbose and hard
to change.  In other cases in recent memory, I've wanted to change
direct accessors to separate assignments from uses; and this requires
global changes anyway, meaning that the macros didn't really help all
that much.

Tom

Re: FYI: use SYMBOL_SYMTAB accessor

[Joel Brobecker]

> I'm somewhat on the fence about this, but on balance I think it is a
> good idea to make types that are used in more than one file opaque.
> 
> However, in practice this hasn't always helped much.  The symbol
> accessors are the worst, as we've discussed before, because many of them
> are polymorphic without any purpose -- so they are both verbose and hard
> to change.  In other cases in recent memory, I've wanted to change
> direct accessors to separate assignments from uses; and this requires
> global changes anyway, meaning that the macros didn't really help all
> that much.

Yeah - in retrospect, I think we should have required that for our
types. I like opaque types (for complex types), because it allows
us to control the contents of the data through a well defined API,
and then allows us to easily track its use. Our macros serve this
purpose to a degree, except that we do not automatically catch
mistakes where we access the field directly instead of through
the macros.

Anyway, I don't think it's really all that important for existing
code at the moment. But something perhaps to bear in mind if we
ever create new data structures - to ask ourselves whether the type
should be opaque or not. Or if we revamp our symbol/types data.

-- 
Joel