[PATCH] Kill TYPE_METHODS 0/9

[PATCH] Kill TYPE_METHODS 0/9

[Nathan Sidwell]

This is a series of patches that remove the TYPE_METHODS field used in records & 
unions.  Currently TYPE_METHODS hods a the member functions (be they static or 
non-static), and TYPE_FIELDS holds everything else (be they FIELD_DECLS or 
whatever).  This distinction is unnecessary, and the patches move everything to 
TYPE_FIELDS.  (I do not mess with name lookup, which is handled differently).

I do not repurpose TYPE_METHODS, that's later.

While the changes are pretty mechanical, some are rather too large outside of 
the C++ FE to comfortably apply the obvious rule.

1 method-debug.diff - dbxout & dwarf2out.  Review please.
2 method-ipa.diff - lto-devirt.  Review please.
3 method-rtl.diff - most odd occurrence.  Comment please.

4 method-ada.diff - ada-spec generation.  Obvious.
5 method-cp.diff - C++ FE changes, self reviewed
6 method-libcc1.diff - libcp1plugin.  Obvious.
7 method-misc.diff - random tree.c.  Obvious.
8 method-objc.diff - objc.  Obvious

9 method-ectomy.diff - delete the macro.  Obvious

nathan

-- 
Nathan Sidwell

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

This changes dbxout and dwarf2out.

Rather than iterate over the TYPE_METHODS, they now need to deal with member fns 
in the regular TYPE_FIELDS iteration.
dbxout was a little weirdly convoluted, apparently presuming that functions with 
the same name are all together.  That's not true, so other than maybe slight 
debug bloat in cases when they happen to be adjacent, it seems more sensible to 
handle each member function as a separate item.

The dwarf2out changes are just moving the processing to the TYPE_FIELDS loop, 
and thereby deleting some duplicate code.

I'd appreciate a review of this patch.

Oh, the patch series survived a bootstrap on x86_64-linux.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-debug.diff --]
[-- Type: text/plain, Size: 6356 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* dbxout.c (dbxout_type_fields): Member fns are on TYPE_FIELDS.
	(dbxout_type_method_1, dbxout_type_methods): Delete.
	(dbxout_type_fn_member): New, constructed from previous.
	(dbxout_type): No TYPE_METHODS scan.
	* dwarf2out.c (gen_member_die): Member fns are on TYPE_FIELDS.

Index: gcc/dbxout.c
===================================================================
--- gcc/dbxout.c	(revision 250160)
+++ gcc/dbxout.c	(working copy)
@@ -311,8 +311,7 @@ static void dbxout_typedefs (tree);
 static void dbxout_type_index (tree);
 static void dbxout_args (tree);
 static void dbxout_type_fields (tree);
-static void dbxout_type_method_1 (tree);
-static void dbxout_type_methods (tree);
+static void dbxout_type_fn_member (tree);
 static void dbxout_range_type (tree, tree, tree);
 static void dbxout_type (tree, int);
 static bool print_int_cst_bounds_in_octal_p (tree, tree, tree);
@@ -1493,6 +1492,8 @@ dbxout_type_fields (tree type)
 		  || ! tree_fits_uhwi_p (DECL_SIZE (tem)))))
 	continue;
 
+      else if (TREE_CODE (tem) == FUNCTION_DECL)
+	dbxout_type_fn_member (tem);
       else if (TREE_CODE (tem) != CONST_DECL)
 	{
 	  /* Continue the line if necessary,
@@ -1542,14 +1543,23 @@ dbxout_type_fields (tree type)
     }
 }
 \f
-/* Subroutine of `dbxout_type_methods'.  Output debug info about the
-   method described DECL.  */
+/* Subroutine of `dbxout_type'.  Output debug info about the
+   function member DECL.  */
 
 static void
-dbxout_type_method_1 (tree decl)
+dbxout_type_fn_member (tree decl)
 {
+  if (!use_gnu_debug_info_extensions)
+    return;
+
   char c1 = 'A', c2;
 
+  CONTIN;
+  stabstr_I (DECL_NAME (decl));
+  stabstr_S ("::");
+
+  dbxout_type (TREE_TYPE (decl), 0);
+
   if (TREE_CODE (TREE_TYPE (decl)) == FUNCTION_TYPE)
     c2 = '?';
   else /* it's a METHOD_TYPE.  */
@@ -1586,72 +1596,7 @@ dbxout_type_method_1 (tree decl)
       dbxout_type (DECL_CONTEXT (decl), 0);
       stabstr_C (';');
     }
-}
-\f
-/* Subroutine of `dbxout_type'.  Output debug info about the methods defined
-   in TYPE.  */
-
-static void
-dbxout_type_methods (tree type)
-{
-  /* C++: put out the method names and their parameter lists */
-  tree methods = TYPE_METHODS (type);
-  tree fndecl;
-  tree last;
-
-  if (methods == NULL_TREE)
-    return;
-
-  if (TREE_CODE (methods) != TREE_VEC)
-    fndecl = methods;
-  else if (TREE_VEC_ELT (methods, 0) != NULL_TREE)
-    fndecl = TREE_VEC_ELT (methods, 0);
-  else
-    fndecl = TREE_VEC_ELT (methods, 1);
-
-  while (fndecl)
-    {
-      int need_prefix = 1;
-
-      /* Group together all the methods for the same operation.
-	 These differ in the types of the arguments.  */
-      for (last = NULL_TREE;
-	   fndecl && (last == NULL_TREE || DECL_NAME (fndecl) == DECL_NAME (last));
-	   fndecl = DECL_CHAIN (fndecl))
-	/* Output the name of the field (after overloading), as
-	   well as the name of the field before overloading, along
-	   with its parameter list */
-	{
-	  /* Skip methods that aren't FUNCTION_DECLs.  (In C++, these
-	     include TEMPLATE_DECLs.)  The debugger doesn't know what
-	     to do with such entities anyhow.  */
-	  if (TREE_CODE (fndecl) != FUNCTION_DECL)
-	    continue;
-
-	  CONTIN;
-
-	  last = fndecl;
-
-	  /* Also ignore abstract methods; those are only interesting to
-	     the DWARF backends.  */
-	  if (DECL_IGNORED_P (fndecl) || DECL_ABSTRACT_P (fndecl))
-	    continue;
-
-	  /* Redundantly output the plain name, since that's what gdb
-	     expects.  */
-	  if (need_prefix)
-	    {
-	      stabstr_I (DECL_NAME (fndecl));
-	      stabstr_S ("::");
-	      need_prefix = 0;
-	    }
-
-	  dbxout_type (TREE_TYPE (fndecl), 0);
-	  dbxout_type_method_1 (fndecl);
-	}
-      if (!need_prefix)
-	stabstr_C (';');
-    }
+  stabstr_C (';');
 }
 
 /* Emit a "range" type specification, which has the form:
@@ -2211,10 +2156,6 @@ dbxout_type (tree type, int full)
 
       /* Write out the field declarations.  */
       dbxout_type_fields (type);
-      if (use_gnu_debug_info_extensions && TYPE_METHODS (type) != NULL_TREE)
-	{
-	  dbxout_type_methods (type);
-	}
 
       stabstr_C (';');
 
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 250160)
+++ gcc/dwarf2out.c	(working copy)
@@ -24088,7 +24088,8 @@ gen_member_die (tree type, dw_die_ref co
 {
   tree member;
   tree binfo = TYPE_BINFO (type);
-  dw_die_ref child;
+
+  gcc_assert (TYPE_MAIN_VARIANT (type) == type);
 
   /* If this is not an incomplete type, output descriptions of each of its
      members. Note that as we output the DIEs necessary to represent the
@@ -24125,13 +24126,16 @@ gen_member_die (tree type, dw_die_ref co
 	   && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline)
 	       != -1));
 
+      /* Ignore clones.  */
+      if (DECL_ABSTRACT_ORIGIN (member))
+	continue;
+
       /* If we thought we were generating minimal debug info for TYPE
 	 and then changed our minds, some of the member declarations
 	 may have already been defined.  Don't define them again, but
 	 do put them in the right order.  */
 
-      child = lookup_decl_die (member);
-      if (child)
+      if (dw_die_ref child = lookup_decl_die (member))
 	{
 	  /* Handle inline static data members, which only have in-class
 	     declarations.  */
@@ -24159,6 +24163,7 @@ gen_member_die (tree type, dw_die_ref co
 		  static_inline_p = false;
 		}
 	    }
+
 	  if (child->die_tag == DW_TAG_variable
 	      && child->die_parent == comp_unit_die ()
 	      && ref == NULL)
@@ -24197,23 +24202,6 @@ gen_member_die (tree type, dw_die_ref co
 	  DECL_EXTERNAL (member) = old_extern;
 	}
     }
-
-  /* We do not keep type methods in type variants.  */
-  gcc_assert (TYPE_MAIN_VARIANT (type) == type);
-  /* Now output info about the function members (if any).  */
-  if (TYPE_METHODS (type) != error_mark_node)
-    for (member = TYPE_METHODS (type); member; member = DECL_CHAIN (member))
-      {
-	/* Don't include clones in the member list.  */
-	if (DECL_ABSTRACT_ORIGIN (member))
-	  continue;
-
-	child = lookup_decl_die (member);
-	if (child)
-	  splice_child_die (context_die, child);
-	else
-	  gen_decl_die (member, NULL, NULL, context_die);
-      }
 }
 
 /* Generate a DIE for a structure or union type.  If TYPE_DECL_SUPPRESS_DEBUG

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Jim Wilson]

On 07/14/2017 09:48 AM, Nathan Sidwell wrote:
> This changes dbxout and dwarf2out.

> Oh, the patch series survived a bootstrap on x86_64-linux.

Changes to the debug info files requires a gdb make check with and 
without the patch to check for regressions.  Since you are changing both 
dbxout and dwarf2out, you would need to do this twice, once for each 
debug info type.  Testing dbxout may be a little tricky, since few 
systems still use this by default.  Maybe you can hack an x86_64-linux 
build to use dbxout by default to test it.

Otherwise, this looks OK.

Jim

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Nathan Sidwell]

On 07/18/2017 01:24 PM, Jim Wilson wrote:

> Changes to the debug info files requires a gdb make check with and 
> without the patch to check for regressions.  Since you are changing both 
> dbxout and dwarf2out, you would need to do this twice, once for each 
> debug info type.  Testing dbxout may be a little tricky, since few 
> systems still use this by default.  Maybe you can hack an x86_64-linux 
> build to use dbxout by default to test it.

DWARF results are unchanged.

DBX results after hacking x86-64 to prefer stabs are too horrible to 
tell.  However, a manual check on a simple program shows that gdb 
couldn't locate member functions anyway even before the patch and an 
unhacked g++.

g++ -gstabs:
(gdb) ptype X
type = struct X {
     int m;
}

g++ -gdwarf:
(gdb) ptype X
type = struct X {
     int m;
   public:
     int frob(int);
     X(int);
}

So I don't think I've made it worse there.   thoughts?

nathan

-- 
Nathan Sidwell

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Nathan Sidwell]

On 07/20/2017 07:31 AM, Nathan Sidwell wrote:

> So I don't think I've made it worse there.   thoughts?

aha, gstabs+ for gnu extensions. With a small fix I get the following 
before and after (I changed the example to add another frob member).
(gdb) ptype X
type = struct X {
   public:
     int frob(int);
     int frob(float);
     X(int);
     X(int);
}

The fragment I'd missed is losing:
   /* Also ignore abstract methods; those are only interesting to
      the DWARF backends.  */
   if (DECL_IGNORED_P (decl) || DECL_ABSTRACT_P (decl))
     return;

which inhibits the abstract ctor.

nathan

-- 
Nathan Sidwell

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

A bit more poking showed that dbx's desire to put all methods of the 
same name in a single record was significant -- gdb requires that to 
build an overload set.

So, this version of the dbxout changes is much smaller, in that we 
iterate the TYPE_FIELDS twice.  There's the original scan, which just 
needs to now skip FUNCTION_DECLs as well.  The method emission routine 
now iterates over TYPE_FIELDS again, skipping everything that is not a 
FUNCTION_DECL.  I preserve the previous behaviour of merging decls with 
the same name -- of course it'll still fail in the same manner if you 
declare overloads discontigously.

With this patch the gdb stabs test results are still awful, but they are 
unchanged awfulness.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: 1a-method-dbx.diff --]
[-- Type: text/x-patch, Size: 3042 bytes --]

2017-07-20  Nathan Sidwell  <nathan@acm.org>

	* dbxout.c (dbxout_type_fields): Ignore FUNCTION_DECLs.
	(dbxout_type_methods): Scan TYPE_FIELDS.
	(dbxout_type): Don't check TYPE_METHODS here.

Index: dbxout.c
===================================================================
--- dbxout.c	(revision 250318)
+++ dbxout.c	(working copy)
@@ -1481,6 +1481,8 @@ dbxout_type_fields (tree type)
       /* Omit here local type decls until we know how to support them.  */
       if (TREE_CODE (tem) == TYPE_DECL
 	  || TREE_CODE (tem) == TEMPLATE_DECL
+	  /* Member functions emitted after fields.  */
+	  || TREE_CODE (tem) == FUNCTION_DECL
 	  /* Omit here the nameless fields that are used to skip bits.  */
 	  || DECL_IGNORED_P (tem)
 	  /* Omit fields whose position or size are variable or too large to
@@ -1586,55 +1588,38 @@ dbxout_type_method_1 (tree decl)
     }
 }
 \f
-/* Subroutine of `dbxout_type'.  Output debug info about the methods defined
-   in TYPE.  */
+/* Subroutine of `dbxout_type'.  Output debug info about the member
+   functions defined in TYPE.  */
 
 static void
 dbxout_type_methods (tree type)
 {
-  /* C++: put out the method names and their parameter lists */
-  tree methods = TYPE_METHODS (type);
-  tree fndecl;
-  tree last;
-
-  if (methods == NULL_TREE)
-    return;
-
-  if (TREE_CODE (methods) != TREE_VEC)
-    fndecl = methods;
-  else if (TREE_VEC_ELT (methods, 0) != NULL_TREE)
-    fndecl = TREE_VEC_ELT (methods, 0);
-  else
-    fndecl = TREE_VEC_ELT (methods, 1);
-
-  while (fndecl)
+  for (tree fndecl = TYPE_FIELDS (type); fndecl;)
     {
       int need_prefix = 1;
 
       /* Group together all the methods for the same operation.
 	 These differ in the types of the arguments.  */
-      for (last = NULL_TREE;
+      for (tree last = NULL_TREE;
 	   fndecl && (last == NULL_TREE || DECL_NAME (fndecl) == DECL_NAME (last));
 	   fndecl = DECL_CHAIN (fndecl))
 	/* Output the name of the field (after overloading), as
 	   well as the name of the field before overloading, along
 	   with its parameter list */
 	{
-	  /* Skip methods that aren't FUNCTION_DECLs.  (In C++, these
-	     include TEMPLATE_DECLs.)  The debugger doesn't know what
-	     to do with such entities anyhow.  */
+	  /* Skip non-functions.  */
 	  if (TREE_CODE (fndecl) != FUNCTION_DECL)
 	    continue;
 
-	  CONTIN;
-
-	  last = fndecl;
-
 	  /* Also ignore abstract methods; those are only interesting to
 	     the DWARF backends.  */
 	  if (DECL_IGNORED_P (fndecl) || DECL_ABSTRACT_P (fndecl))
 	    continue;
 
+	  CONTIN;
+
+	  last = fndecl;
+
 	  /* Redundantly output the plain name, since that's what gdb
 	     expects.  */
 	  if (need_prefix)
@@ -2209,10 +2194,8 @@ dbxout_type (tree type, int full)
 
       /* Write out the field declarations.  */
       dbxout_type_fields (type);
-      if (use_gnu_debug_info_extensions && TYPE_METHODS (type) != NULL_TREE)
-	{
-	  dbxout_type_methods (type);
-	}
+      if (use_gnu_debug_info_extensions)
+	dbxout_type_methods (type);
 
       stabstr_C (';');
 

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Jim Wilson]

On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
> With this patch the gdb stabs test results are still awful, but they are
> unchanged awfulness.

Yes, the stabs support for C++ is poor.  That is one of the reasons
why almost everyone has switched to dwarf2.

I wasn't sure what to make of your last message, so I tried to see if
I could build a toolchain that defaults to stabs so I could look at
this.  I discovered that -freorder-functions doesn't work with stabs
on an elf target, as we get a cross section label subtraction that the
assembler can't compute.  I had to manually disable that optimization.
I also had to disable the x86 specific change to enable
-freorder-blocks-and-partition at -O2.  I managed to get it working
for x86_64-linux and i686-pc-cygwin.  But the fact that -O2 and stabs
don't work together by default anymore suggest that there are no
serious stabs users except on old legacy systems that don't support
named sections, and hence don't support -freorder-functions.

I also noticed that there is a config/i386/gstabs.h file that has been
unused since the openbsd 2 and 3 support was removed last year, and
should be deleted.

Anyways, your new dbxout.c patch looks good.  And maybe we should
think about deprecating the stabs support some day.

Jim

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Nathan Sidwell]

On 07/20/2017 06:03 PM, Jim Wilson wrote:

> I wasn't sure what to make of your last message, so I tried to see if
> I could build a toolchain that defaults to stabs so I could look at
> this.  I discovered that -freorder-functions doesn't work with stabs
> on an elf target, as we get a cross section label subtraction that the
> assembler can't compute.  I had to manually disable that optimization.
> I also had to disable the x86 specific change to enable
> -freorder-blocks-and-partition at -O2.  I managed to get it working
> for x86_64-linux and i686-pc-cygwin.  But the fact that -O2 and stabs
> don't work together by default anymore suggest that there are no
> serious stabs users except on old legacy systems that don't support
> named sections, and hence don't support -freorder-functions.

Sorry if I was unclear.  Indeed a complete build failed building libraries as 
you describe.  But for testing purposes I copied stabs-preferring xg++/xgcc and 
cc1/cc1plus to an dwarf install and pointed gdb at that.  Plus some manual testing.

> Anyways, your new dbxout.c patch looks good.  And maybe we should
> think about deprecating the stabs support some day.

agreed.

I have committed the patch series, thanks all for comments.

nathan

-- 
Nathan Sidwell

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Richard Biener]

On July 21, 2017 12:03:58 AM GMT+02:00, Jim Wilson <jim.wilson@linaro.org> wrote:
>On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> With this patch the gdb stabs test results are still awful, but they
>are
>> unchanged awfulness.
>
>Yes, the stabs support for C++ is poor.  That is one of the reasons
>why almost everyone has switched to dwarf2.
>
>I wasn't sure what to make of your last message, so I tried to see if
>I could build a toolchain that defaults to stabs so I could look at
>this.  I discovered that -freorder-functions doesn't work with stabs
>on an elf target, as we get a cross section label subtraction that the
>assembler can't compute.  I had to manually disable that optimization.
>I also had to disable the x86 specific change to enable
>-freorder-blocks-and-partition at -O2.  I managed to get it working
>for x86_64-linux and i686-pc-cygwin.  But the fact that -O2 and stabs
>don't work together by default anymore suggest that there are no
>serious stabs users except on old legacy systems that don't support
>named sections, and hence don't support -freorder-functions.
>
>I also noticed that there is a config/i386/gstabs.h file that has been
>unused since the openbsd 2 and 3 support was removed last year, and
>should be deleted.
>
>Anyways, your new dbxout.c patch looks good.  And maybe we should
>think about deprecating the stabs support some day.

I've suggested that multiple times also to be able to get rid of the debug hook interfacing in GCC and emit dwarf directly from FEs where that makes sense.  DWARF should be a superset of other debug formats so it should be possible to build stabs from dwarf.

Richard.

>Jim

Deprecate DBX/stabs?

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

[darwin, cygwin, rx maintainers, you might have an opinion]

On 07/21/2017 01:11 AM, Richard Biener wrote:
> On July 21, 2017 12:03:58 AM GMT+02:00, Jim Wilson <jim.wilson@linaro.org> wrote:
>> On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>> With this patch the gdb stabs test results are still awful, but they
>> are
>>> unchanged awfulness.
>>
>> Anyways, your new dbxout.c patch looks good.  And maybe we should
>> think about deprecating the stabs support some day.
> 
> I've suggested that multiple times also to be able to get rid of the debug hook interfacing in GCC and emit dwarf directly from FEs where that makes sense.  DWARF should be a superset of other debug formats so it should be possible to build stabs from dwarf.

Let's at least deprecate it.  I attach a patch to do so.  With the 
patch, you'll get a note about dbx being deprecated whenever you use 
stabs debugging on a system that prefers stabs (thus both -g and -gstabs 
will warn).  On systems where stabs is not preferred, -gstabs will not 
give you a warning.  The patch survices an x86_64-linux bootstrap.

A config can chose to override thus by defining 'DBX_DEBUG_OK' in the 
build defines.

I did try build time CPP tricks, but config/rx.h and 
config/i386/darwin.h define it to be a conditional expression.

AFAICT, the following include files are not used, and could probably be 
binned too:
config/dbx.h
config/dbxcoff.h
config/dbxelf.h
(+ configi386/gstabs.h Jim found)

It looks like DBX is the default for:
i386/cygming configured for 32-bit or lacking PE_SECREL32_RELOC
i386/darwin.h for 32-bit target
rx/rx.h when using AS100 syntax

nathan
-- 
Nathan Sidwell

[-- Attachment #2: dbx-dep.diff --]
[-- Type: text/x-patch, Size: 672 bytes --]

2017-07-21  Nathan Sidwell  <nathan@acm.org>

	* toplev.c (process_options): Warn about DBX being deprecated.

Index: toplev.c
===================================================================
--- toplev.c	(revision 250424)
+++ toplev.c	(working copy)
@@ -1413,6 +1413,12 @@ process_options (void)
 	debug_info_level = DINFO_LEVEL_NONE;
     }
 
+#ifndef DBX_DEBBUG_OK
+  if (PREFERRED_DEBUGGING_TYPE == DBX_DEBUG
+      && write_symbols == DBX_DEBUG)
+    inform (UNKNOWN_LOCATION, "DBX (stabs) debugging is deprecated");
+#endif
+
   if (flag_dump_final_insns && !flag_syntax_only && !no_backend)
     {
       FILE *final_output = fopen (flag_dump_final_insns, "w");

Re: Deprecate DBX/stabs?

[Richard Biener]

On Fri, Jul 21, 2017 at 3:07 PM, Nathan Sidwell <nathan@acm.org> wrote:
> [darwin, cygwin, rx maintainers, you might have an opinion]
>
> On 07/21/2017 01:11 AM, Richard Biener wrote:
>>
>> On July 21, 2017 12:03:58 AM GMT+02:00, Jim Wilson <jim.wilson@linaro.org>
>> wrote:
>>>
>>> On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>>>
>>>> With this patch the gdb stabs test results are still awful, but they
>>>
>>> are
>>>>
>>>> unchanged awfulness.
>>>
>>>
>>> Anyways, your new dbxout.c patch looks good.  And maybe we should
>>> think about deprecating the stabs support some day.
>>
>>
>> I've suggested that multiple times also to be able to get rid of the debug
>> hook interfacing in GCC and emit dwarf directly from FEs where that makes
>> sense.  DWARF should be a superset of other debug formats so it should be
>> possible to build stabs from dwarf.
>
>
> Let's at least deprecate it.  I attach a patch to do so.  With the patch,
> you'll get a note about dbx being deprecated whenever you use stabs
> debugging on a system that prefers stabs (thus both -g and -gstabs will
> warn).  On systems where stabs is not preferred, -gstabs will not give you a
> warning.  The patch survices an x86_64-linux bootstrap.
>
> A config can chose to override thus by defining 'DBX_DEBUG_OK' in the build
> defines.
>
> I did try build time CPP tricks, but config/rx.h and config/i386/darwin.h
> define it to be a conditional expression.
>
> AFAICT, the following include files are not used, and could probably be
> binned too:
> config/dbx.h
> config/dbxcoff.h
> config/dbxelf.h
> (+ configi386/gstabs.h Jim found)
>
> It looks like DBX is the default for:
> i386/cygming configured for 32-bit or lacking PE_SECREL32_RELOC
> i386/darwin.h for 32-bit target
> rx/rx.h when using AS100 syntax

Index: toplev.c
===================================================================
--- toplev.c    (revision 250424)
+++ toplev.c    (working copy)
@@ -1413,6 +1413,12 @@ process_options (void)
        debug_info_level = DINFO_LEVEL_NONE;
     }

+#ifndef DBX_DEBBUG_OK
                        ^^^
typo?  The patch doesn't define this anywhere - I suggest to add it to
defaults.h
as 0 and use #if?  Also would need documenting if this is supposed to be a
target macro.

Thanks,
Richard.

> nathan
> --
> Nathan Sidwell

Re: Deprecate DBX/stabs?

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

On 07/21/2017 09:16 AM, Richard Biener wrote:
> On Fri, Jul 21, 2017 at 3:07 PM, Nathan Sidwell <nathan@acm.org> wrote:

> +#ifndef DBX_DEBBUG_OK
>                          ^^^
> typo?  The patch doesn't define this anywhere - I suggest to add it to
> defaults.h
> as 0 and use #if?  Also would need documenting if this is supposed to be a
> target macro.

Like this?  I've now included XCOFF, as it's a subset of DBX.  Nothing 
appears to default to it.

nathan
-- 
Nathan Sidwell

[-- Attachment #2: dbx-dep.diff --]
[-- Type: text/x-patch, Size: 2976 bytes --]

2017-07-21  Nathan Sidwell  <nathan@acm.org>

	* defaults.h (DBX_DEBUG_DEPRECATED): New.
	* toplev.c (process_options): Warn about DBX/SDB being deprecated.
	* doc/tm.texi.in (DBX_DEBUG_DEPRECATED): Document.
	* doc/tm.texi: Updated.

Index: defaults.h
===================================================================
--- defaults.h	(revision 250426)
+++ defaults.h	(working copy)
@@ -889,6 +889,12 @@ see the files COPYING3 and COPYING.RUNTI
 #define SDB_DEBUGGING_INFO 0
 #endif
 
+/* DBX debugging is deprecated, and will generate a note if you
+   default to it.  */
+#ifndef DBX_DEBUG_DEPRECATED
+#define DBX_DEBUG_DEPRECATED 1
+#endif
+
 /* If more than one debugging type is supported, you must define
    PREFERRED_DEBUGGING_TYPE to choose the default.  */
 
Index: doc/tm.texi
===================================================================
--- doc/tm.texi	(revision 250426)
+++ doc/tm.texi	(working copy)
@@ -9553,6 +9553,14 @@ user can always get a specific type of o
 
 @c prevent bad page break with this line
 These are specific options for DBX output.
+DBX debug data is deprecated and is expected to be removed.
+
+@defmac DBX_DEBUG_DEPRECATED
+Defined this macro to 1 if GCC should not warn about defaulting to DBX
+or XCOFF debug output.  This is intended to give maintainers notice of
+deprecation, but not be unnecessarily invasive.  Defining this macro is
+a short-term measure.  You need to plan for DBX's removal.
+@end defmac
 
 @defmac DBX_DEBUGGING_INFO
 Define this macro if GCC should produce debugging output for DBX
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 250426)
+++ doc/tm.texi.in	(working copy)
@@ -6842,6 +6842,14 @@ user can always get a specific type of o
 
 @c prevent bad page break with this line
 These are specific options for DBX output.
+DBX debug data is deprecated and is expected to be removed.
+
+@defmac DBX_DEBUG_DEPRECATED
+Defined this macro to 1 if GCC should not warn about defaulting to DBX
+or XCOFF debug output.  This is intended to give maintainers notice of
+deprecation, but not be unnecessarily invasive.  Defining this macro is
+a short-term measure.  You need to plan for DBX's removal.
+@end defmac
 
 @defmac DBX_DEBUGGING_INFO
 Define this macro if GCC should produce debugging output for DBX
Index: toplev.c
===================================================================
--- toplev.c	(revision 250426)
+++ toplev.c	(working copy)
@@ -1413,6 +1413,12 @@ process_options (void)
 	debug_info_level = DINFO_LEVEL_NONE;
     }
 
+  if (DBX_DEBUG_DEPRECATED
+      && write_symbols == PREFERRED_DEBUGGING_TYPE
+      && (PREFERRED_DEBUGGING_TYPE == DBX_DEBUG
+	  || PREFERRED_DEBUGGING_TYPE == XCOFF_DEBUG))
+    inform (UNKNOWN_LOCATION, "DBX/XCOFF (stabs) debugging is deprecated");
+
   if (flag_dump_final_insns && !flag_syntax_only && !no_backend)
     {
       FILE *final_output = fopen (flag_dump_final_insns, "w");

Re: Deprecate DBX/stabs?

[Mike Stump]

On Jul 21, 2017, at 6:07 AM, Nathan Sidwell <nathan@acm.org> wrote:
> 
> [darwin, cygwin, rx maintainers, you might have an opinion]

darwin going forward is a DWARF platform, so, shouldn't be a heartache for real folks.  For ancient machines, ancient compilers might be a requirement.  Generally, I like keeping things; but cleanups and removals are a part of life, and eventually the old should be removed. I'd not stand in the way of such removal.  If we were within 5 years of such a transition point, I'd argue to keep it for at least 5 years.  But, the switch for darwin was Oct 26th, 2007.  10 years I think is a nice cutover point for first tier things.  Beyond 10, and I'd say, you are dragging your feet.  If _all_ the code for DBX were in my port file, I'd be tempted to keep it indefinitely.  It's not, so, that's not possible/reasonable.

Iain, do you still have the G5s?  :-)  Do they run 8 or 9?  What do you think?  Seem reasonable?

The 64_BIT x86 darwin question likely can be be flipped to default to dwarf; so, even that isn't an issue.

Re: Deprecate DBX/stabs?

[Iain Sandoe]

> On 21 Jul 2017, at 20:10, Mike Stump <mikestump@comcast.net> wrote:
> 
> On Jul 21, 2017, at 6:07 AM, Nathan Sidwell <nathan@acm.org> wrote:
>> 
>> [Darwin, cygwin, rx maintainers, you might have an opinion]
> 
> darwin going forward is a DWARF platform, so, shouldn't be a heartache for real folks.

agreed,
in fact the default for current assemblers doesn’t support stabs - and we’ve adjusted the specs to deal with that.

>  For ancient machines, ancient compilers might be a requirement.  Generally, I like keeping things; but cleanups and removals are a part of life, and eventually the old should be removed. I'd not stand in the way of such removal.  If we were within 5 years of such a transition point, I'd argue to keep it for at least 5 years.  But, the switch for darwin was Oct 26th, 2007.  10 years I think is a nice cutover point for first tier things.  Beyond 10, and I'd say, you are dragging your feet.  If _all_ the code for DBX were in my port file, I'd be tempted to keep it indefinitely.  It's not, so, that's not possible/reasonable.
> 
> Iain, do you still have the G5s?  :-)  Do they run 8 or 9?  What do you think?  Seem reasonable?

I still have access to i686 and powerpc Darwin8, but testing is extremely infrequent.

For the record; anyone wanting modern toolchains on Darwin8 most likely has to face building a linker and more modern cctools anyway(the XCode 2.5 set was extremely flaky from at least 4.6/4.7). 

I’d suspect that a person serious in doing that would likely be willing to build GDB which does support x86 Darwin, at least.  FWIW I have a patch that makes GDB (at least 7.x) work for PowerPC too.  If anyone cares, ask ;-) (I doubt if I’ll try modernising it across the transition to c++ for GDB - since available time would prob be better spent elsewhere).

Anyone wanting to build modern GCC on < Darwin8 is going to need to build most of the supporting infra too - the (XCode 1.5) linker/assembler there don’t support enough weak stuff to be viable for modern c++.  These very old platforms are long past the “config && make” stage, supporting them needs a degree of commitment and understanding.

-----

My G5 ( and most of my other ppc hardware) habitually runs 10.5 (Darwin9) and I’ve no motivation to reboot to 10.4 unless to test something more quickly than my really ancient G4s can manage.

> The 64_BIT x86 darwin question likely can be be flipped to default to dwarf; so, even that isn't an issue.

It ought to be already, in fact anything (powerpc*/x86/x86-64) >= Darwin9 (OS X 10.5) ought to be defaulting to DWARF already, will check that sometime.

cheers,
Iain
Iain Sandoe
CodeSourcery / Mentor Embedded

Re: Deprecate DBX/stabs?

[Jim Wilson]

On Fri, Jul 21, 2017 at 12:44 PM, Iain Sandoe <iain@codesourcery.com> wrote:
> It ought to be already, in fact anything (powerpc*/x86/x86-64) >= Darwin9 (OS X 10.5) ought to be defaulting to DWARF already, will check that sometime.

Yes, they do default to dwarf2.  The comments say pre-darwin9 32-bit
defaults to stabs.  The question is whether anyone cares about that
anymore.

Jim

Re: Deprecate DBX/stabs?

[Iain Sandoe]

> On 21 Jul 2017, at 20:54, Jim Wilson <jim.wilson@linaro.org> wrote:
> 
> On Fri, Jul 21, 2017 at 12:44 PM, Iain Sandoe <iain@codesourcery.com> wrote:
>> It ought to be already, in fact anything (powerpc*/x86/x86-64) >= Darwin9 (OS X 10.5) ought to be defaulting to DWARF already, will check that sometime.
> 
> Yes, they do default to dwarf2.  The comments say pre-darwin9 32-bit
> defaults to stabs.  The question is whether anyone cares about that
> anymore.

From my perspective, as per Mike’s comments, I’d say “let’s move on”,
- Darwin8’s DWARF support is good enough for C at least
- as per my other comments, there remain ways forward for someone who really wants to support it (TenFourFox seems still active and I do get a few queries per year from folks working with Darwin8).
- deprecation gives other folks a chance to shout if they care.

cheers
Iain

Iain Sandoe
CodeSourcery / Mentor Embedded

Re: Deprecate DBX/stabs?

[JonY]

[-- Attachment #1.1: Type: text/plain, Size: 1168 bytes --]

On 07/21/2017 01:07 PM, Nathan Sidwell wrote:
> [darwin, cygwin, rx maintainers, you might have an opinion]
> Let's at least deprecate it.  I attach a patch to do so.  With the
> patch, you'll get a note about dbx being deprecated whenever you use
> stabs debugging on a system that prefers stabs (thus both -g and -gstabs
> will warn).  On systems where stabs is not preferred, -gstabs will not
> give you a warning.  The patch survices an x86_64-linux bootstrap.
> 
> A config can chose to override thus by defining 'DBX_DEBUG_OK' in the
> build defines.
> 
> I did try build time CPP tricks, but config/rx.h and
> config/i386/darwin.h define it to be a conditional expression.
> 
> AFAICT, the following include files are not used, and could probably be
> binned too:
> config/dbx.h
> config/dbxcoff.h
> config/dbxelf.h
> (+ configi386/gstabs.h Jim found)
> 
> It looks like DBX is the default for:
> i386/cygming configured for 32-bit or lacking PE_SECREL32_RELOC
> i386/darwin.h for 32-bit target
> rx/rx.h when using AS100 syntax
> 
> nathan

Cygwin GCC has been using --with-dwarf2 for sometime now, so it
shouldn't be affected.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

Re: [PATCH] Kill TYPE_METHODS debug 1/9

[Jeff Law]

On 07/20/2017 04:03 PM, Jim Wilson wrote:
> On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> With this patch the gdb stabs test results are still awful, but they are
>> unchanged awfulness.
> 
> Yes, the stabs support for C++ is poor.  That is one of the reasons
> why almost everyone has switched to dwarf2.
> 
> I wasn't sure what to make of your last message, so I tried to see if
> I could build a toolchain that defaults to stabs so I could look at
> this.  I discovered that -freorder-functions doesn't work with stabs
> on an elf target, 
Hmm, it's supposed to if we support named sections.  But clearly the
code has bit-rotted through the years.  One more strike against stabs ;-)


> 
> I also noticed that there is a config/i386/gstabs.h file that has been
> unused since the openbsd 2 and 3 support was removed last year, and
> should be deleted.
Pre-approved :-)

> 
> Anyways, your new dbxout.c patch looks good.  And maybe we should
> think about deprecating the stabs support some day.
Agreed.  I think the biggest concern is AIX.

jeff

Re: [PATCH] Kill TYPE_METHODS ipa 2/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

This patch changes ipa-devirt.  It contains a loop checking all the member 
functions are 'the same'.  However, as the comment says, we've already zapped 
TYPE_METHODS, so the loop is never entered.   I've added equivalent zapping in 
this patch series, as the rationale appears to be reducing memory footprint. 
It didn't seem appropriate to merge into the TYPE_FIELDS loop, given it's never 
exercised.

I'd appreciate review.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-ipa.diff --]
[-- Type: text/plain, Size: 2478 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* ipa-devirt.c (odr_types_equivalent_p): Delete TYPE_METHODS scan.

Index: gcc/ipa-devirt.c
===================================================================
--- gcc/ipa-devirt.c	(revision 250160)
+++ gcc/ipa-devirt.c	(working copy)
@@ -1602,62 +1602,6 @@ odr_types_equivalent_p (tree t1, tree t2
 		
 		return false;
 	      }
-	    if ((TYPE_MAIN_VARIANT (t1) == t1 || TYPE_MAIN_VARIANT (t2) == t2)
-		&& COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t1))
-		&& COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t2))
-		&& odr_type_p (TYPE_MAIN_VARIANT (t1))
-		&& odr_type_p (TYPE_MAIN_VARIANT (t2))
-		&& (TYPE_METHODS (TYPE_MAIN_VARIANT (t1))
-		    != TYPE_METHODS (TYPE_MAIN_VARIANT (t2))))
-	      {
-		/* Currently free_lang_data sets TYPE_METHODS to error_mark_node
-		   if it is non-NULL so this loop will never realy execute.  */
-		if (TYPE_METHODS (TYPE_MAIN_VARIANT (t1)) != error_mark_node
-		    && TYPE_METHODS (TYPE_MAIN_VARIANT (t2)) != error_mark_node)
-		  for (f1 = TYPE_METHODS (TYPE_MAIN_VARIANT (t1)),
-		       f2 = TYPE_METHODS (TYPE_MAIN_VARIANT (t2));
-		       f1 && f2 ; f1 = DECL_CHAIN (f1), f2 = DECL_CHAIN (f2))
-		    {
-		      if (DECL_ASSEMBLER_NAME (f1) != DECL_ASSEMBLER_NAME (f2))
-			{
-			  warn_odr (t1, t2, f1, f2, warn, warned,
-				    G_("a different method of same type "
-				       "is defined in another "
-				       "translation unit"));
-			  return false;
-			}
-		      if (DECL_VIRTUAL_P (f1) != DECL_VIRTUAL_P (f2))
-			{
-			  warn_odr (t1, t2, f1, f2, warn, warned,
-				    G_("a definition that differs by virtual "
-				       "keyword in another translation unit"));
-			  return false;
-			}
-		      if (DECL_VINDEX (f1) != DECL_VINDEX (f2))
-			{
-			  warn_odr (t1, t2, f1, f2, warn, warned,
-				    G_("virtual table layout differs "
-				       "in another translation unit"));
-			  return false;
-			}
-		      if (odr_subtypes_equivalent_p (TREE_TYPE (f1),
-						     TREE_TYPE (f2), visited,
-						     loc1, loc2))
-			{
-			  warn_odr (t1, t2, f1, f2, warn, warned,
-				    G_("method with incompatible type is "
-				       "defined in another translation unit"));
-			  return false;
-			}
-		    }
-		if ((f1 == NULL) != (f2 == NULL))
-		  {
-		    warn_odr (t1, t2, NULL, NULL, warn, warned,
-			      G_("a type with different number of methods "
-				 "is defined in another translation unit"));
-		    return false;
-		  }
-	      }
 	  }
 	break;
       }

Re: [PATCH] Kill TYPE_METHODS ipa 2/9

[Jan Hubicka]

> This patch changes ipa-devirt.  It contains a loop checking all the member
> functions are 'the same'.  However, as the comment says, we've already
> zapped TYPE_METHODS, so the loop is never entered.   I've added equivalent
> zapping in this patch series, as the rationale appears to be reducing memory
> footprint. It didn't seem appropriate to merge into the TYPE_FIELDS loop,
> given it's never exercised.
> 
> I'd appreciate review.
> 
> nathan
> 
> -- 
> Nathan Sidwell

> 2017-07-14  Nathan Sidwell  <nathan@acm.org>
> 
> 	gcc/
> 	* ipa-devirt.c (odr_types_equivalent_p): Delete TYPE_METHODS scan.

OK (we get virtual methods checked for match by comparing virtual tables,
methods of same name are ODR checked, so we only miss diagnostics when 
one unit defines non-virtual method while other doesn't.  It would be nice
to check this, too, but there are cheaper ways of doing this)

Honza
> 
> Index: gcc/ipa-devirt.c
> ===================================================================
> --- gcc/ipa-devirt.c	(revision 250160)
> +++ gcc/ipa-devirt.c	(working copy)
> @@ -1602,62 +1602,6 @@ odr_types_equivalent_p (tree t1, tree t2
>  		
>  		return false;
>  	      }
> -	    if ((TYPE_MAIN_VARIANT (t1) == t1 || TYPE_MAIN_VARIANT (t2) == t2)
> -		&& COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t1))
> -		&& COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t2))
> -		&& odr_type_p (TYPE_MAIN_VARIANT (t1))
> -		&& odr_type_p (TYPE_MAIN_VARIANT (t2))
> -		&& (TYPE_METHODS (TYPE_MAIN_VARIANT (t1))
> -		    != TYPE_METHODS (TYPE_MAIN_VARIANT (t2))))
> -	      {
> -		/* Currently free_lang_data sets TYPE_METHODS to error_mark_node
> -		   if it is non-NULL so this loop will never realy execute.  */
> -		if (TYPE_METHODS (TYPE_MAIN_VARIANT (t1)) != error_mark_node
> -		    && TYPE_METHODS (TYPE_MAIN_VARIANT (t2)) != error_mark_node)
> -		  for (f1 = TYPE_METHODS (TYPE_MAIN_VARIANT (t1)),
> -		       f2 = TYPE_METHODS (TYPE_MAIN_VARIANT (t2));
> -		       f1 && f2 ; f1 = DECL_CHAIN (f1), f2 = DECL_CHAIN (f2))
> -		    {
> -		      if (DECL_ASSEMBLER_NAME (f1) != DECL_ASSEMBLER_NAME (f2))
> -			{
> -			  warn_odr (t1, t2, f1, f2, warn, warned,
> -				    G_("a different method of same type "
> -				       "is defined in another "
> -				       "translation unit"));
> -			  return false;
> -			}
> -		      if (DECL_VIRTUAL_P (f1) != DECL_VIRTUAL_P (f2))
> -			{
> -			  warn_odr (t1, t2, f1, f2, warn, warned,
> -				    G_("a definition that differs by virtual "
> -				       "keyword in another translation unit"));
> -			  return false;
> -			}
> -		      if (DECL_VINDEX (f1) != DECL_VINDEX (f2))
> -			{
> -			  warn_odr (t1, t2, f1, f2, warn, warned,
> -				    G_("virtual table layout differs "
> -				       "in another translation unit"));
> -			  return false;
> -			}
> -		      if (odr_subtypes_equivalent_p (TREE_TYPE (f1),
> -						     TREE_TYPE (f2), visited,
> -						     loc1, loc2))
> -			{
> -			  warn_odr (t1, t2, f1, f2, warn, warned,
> -				    G_("method with incompatible type is "
> -				       "defined in another translation unit"));
> -			  return false;
> -			}
> -		    }
> -		if ((f1 == NULL) != (f2 == NULL))
> -		  {
> -		    warn_odr (t1, t2, NULL, NULL, warn, warned,
> -			      G_("a type with different number of methods "
> -				 "is defined in another translation unit"));
> -		    return false;
> -		  }
> -	      }
>  	  }
>  	break;
>        }

Re: [PATCH] Kill TYPE_METHODS rtl 3/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 527 bytes --]

This was the most surprising check of TYPE_METHODS.  When not optimizing we use 
the non-nullness of TYPE_METHODS to figure out if we want to place a non BLKmode 
structure into a register.  On the grounds that one can't call a member function 
with a register-located object.

That seems overly enthusiastic -- if we're not optimizing, who cares?

(When we zap TYHPE_METHODS we currently set it to error_mark_node, if it was 
non-null, so that this above check will work).

I'd appreciate comment.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-rtl.diff --]
[-- Type: text/plain, Size: 1104 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* function.c (use_register_for_decl): Always ignore register for
	class types when not optimizing.

Index: gcc/function.c
===================================================================
--- gcc/function.c	(revision 250160)
+++ gcc/function.c	(working copy)
@@ -2218,20 +2218,11 @@ use_register_for_decl (const_tree decl)
   if (!DECL_REGISTER (decl))
     return false;
 
-  switch (TREE_CODE (TREE_TYPE (decl)))
-    {
-    case RECORD_TYPE:
-    case UNION_TYPE:
-    case QUAL_UNION_TYPE:
-      /* When not optimizing, disregard register keyword for variables with
-	 types containing methods, otherwise the methods won't be callable
-	 from the debugger.  */
-      if (TYPE_METHODS (TYPE_MAIN_VARIANT (TREE_TYPE (decl))))
-	return false;
-      break;
-    default:
-      break;
-    }
+  /* When not optimizing, disregard register keyword for types that
+     could have methods, otherwise the methods won't be callable from
+     the debugger.  */
+  if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)))
+    return false;
 
   return true;
 }

Re: [PATCH] Kill TYPE_METHODS rtl 3/9

[Jeff Law]

On 07/14/2017 10:54 AM, Nathan Sidwell wrote:
> This was the most surprising check of TYPE_METHODS.  When not optimizing
> we use the non-nullness of TYPE_METHODS to figure out if we want to
> place a non BLKmode structure into a register.  On the grounds that one
> can't call a member function with a register-located object.
> 
> That seems overly enthusiastic -- if we're not optimizing, who cares?
> 
> (When we zap TYHPE_METHODS we currently set it to error_mark_node, if it
> was non-null, so that this above check will work).
> 
> I'd appreciate comment.
Definitely a surprise.  I was hatching an idea this might be related to
the old stupid.c allocator, but it turns out this code is much newer
than that.

If I'm reading BZ39485 correctly, the issue is that when not optimizing,
we were shoving the value into a register rather than dumping it into
memory.

As a result gdb was unable to do an inferior call to its the object's
methods.

Egad.

jeff

Re: [PATCH] Kill TYPE_METHODS rtl 3/9

[Nathan Sidwell]

On 07/15/2017 12:43 AM, Jeff Law wrote:
> On 07/14/2017 10:54 AM, Nathan Sidwell wrote:
>> This was the most surprising check of TYPE_METHODS.  When not optimizing
>> we use the non-nullness of TYPE_METHODS to figure out if we want to
>> place a non BLKmode structure into a register.  On the grounds that one
>> can't call a member function with a register-located object.
>>
>> That seems overly enthusiastic -- if we're not optimizing, who cares?
>>
>> (When we zap TYHPE_METHODS we currently set it to error_mark_node, if it
>> was non-null, so that this above check will work).

> As a result gdb was unable to do an inferior call to its the object's
> methods.

Right, I realize I failed to mention gdb in the email.  To be clear, what the 
snippet does is NEVER put record types in registers, when not optimizing.

nathan
-- 
Nathan Sidwell

Re: [PATCH] Kill TYPE_METHODS ada-spec 4/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 134 bytes --]

This change to the ada-spec generation teaches it about looking at TYPE_FIELDS. 
quite straight forwards.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-ada.diff --]
[-- Type: text/plain, Size: 5285 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/c-family/
	* c-ada-spec.c (is_tagged_type, has_nontrivial_methods,
	dump_ada_template, print_ada_methods,
	print_ada_declaration): Member fns are on TYPE_FIELDS.

Index: gcc/c-family/c-ada-spec.c
===================================================================
--- gcc/c-family/c-ada-spec.c	(revision 250160)
+++ gcc/c-family/c-ada-spec.c	(working copy)
@@ -1070,16 +1070,11 @@ has_static_fields (const_tree type)
 static bool
 is_tagged_type (const_tree type)
 {
-  tree tmp;
-
   if (!type || !RECORD_OR_UNION_TYPE_P (type))
     return false;
 
-  /* TYPE_METHODS is only set on the main variant.  */
-  type = TYPE_MAIN_VARIANT (type);
-
-  for (tmp = TYPE_METHODS (type); tmp; tmp = TREE_CHAIN (tmp))
-    if (TREE_CODE (tmp) == FUNCTION_DECL && DECL_VINDEX (tmp))
+  for (tree fld = TYPE_FIELDS (type); fld; fld = TREE_CHAIN (fld))
+    if (TREE_CODE (fld) == FUNCTION_DECL && DECL_VINDEX (fld))
       return true;
 
   return false;
@@ -1093,8 +1088,6 @@ is_tagged_type (const_tree type)
 static bool
 has_nontrivial_methods (tree type)
 {
-  tree tmp;
-
   if (!type || !RECORD_OR_UNION_TYPE_P (type))
     return false;
 
@@ -1106,12 +1099,9 @@ has_nontrivial_methods (tree type)
   if (!cpp_check (type, IS_TRIVIAL))
     return true;
 
-  /* TYPE_METHODS is only set on the main variant.  */
-  type = TYPE_MAIN_VARIANT (type);
-
   /* If there are user-defined methods, they are deemed non-trivial.  */
-  for (tmp = TYPE_METHODS (type); tmp; tmp = TREE_CHAIN (tmp))
-    if (!DECL_ARTIFICIAL (tmp))
+  for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
+    if (TREE_CODE (TREE_TYPE (fld)) == METHOD_TYPE && !DECL_ARTIFICIAL (fld))
       return true;
 
   return false;
@@ -1896,7 +1886,7 @@ dump_ada_template (pretty_printer *buffe
       if (TREE_VEC_LENGTH (types) == 0)
 	break;
 
-      if (!RECORD_OR_UNION_TYPE_P (instance) || !TYPE_METHODS (instance))
+      if (!RECORD_OR_UNION_TYPE_P (instance))
 	break;
 
       /* We are interested in concrete template instantiations only: skip
@@ -2442,25 +2432,23 @@ dump_generic_ada_node (pretty_printer *b
 static int
 print_ada_methods (pretty_printer *buffer, tree node, int spc)
 {
-  tree t;
-  int res;
-
   if (!has_nontrivial_methods (node))
     return 0;
 
   pp_semicolon (buffer);
 
-  res = 1;
-  for (t = TYPE_METHODS (node); t; t = TREE_CHAIN (t))
-    {
-      if (res)
-	{
-	  pp_newline (buffer);
-	  pp_newline (buffer);
-	}
-
-      res = print_ada_declaration (buffer, t, node, spc);
-    }
+  int res = 1;
+  for (tree fld = TYPE_FIELDS (node); fld; fld = DECL_CHAIN (fld))
+    if (TREE_CODE (TREE_TYPE (fld)) == METHOD_TYPE)
+      {
+	if (res)
+	  {
+	    pp_newline (buffer);
+	    pp_newline (buffer);
+	  }
+	
+	res = print_ada_declaration (buffer, fld, node, spc);
+      }
 
   return 1;
 }
@@ -2961,19 +2949,13 @@ print_ada_declaration (pretty_printer *b
 	  dump_generic_ada_node (buffer, ret_type, type, spc, false, true);
 	}
 
-      if (is_constructor
-	  && RECORD_OR_UNION_TYPE_P (type)
-	  && TYPE_METHODS (type))
-	{
-	  tree tmp;
-
-	  for (tmp = TYPE_METHODS (type); tmp; tmp = TREE_CHAIN (tmp))
-	    if (cpp_check (tmp, IS_ABSTRACT))
-	      {
-		is_abstract_class = true;
-		break;
-	      }
-	}
+      if (is_constructor && RECORD_OR_UNION_TYPE_P (type))
+	for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
+	  if (cpp_check (fld, IS_ABSTRACT))
+	    {
+	      is_abstract_class = true;
+	      break;
+	    }
 
       if (is_abstract || is_abstract_class)
 	pp_string (buffer, " is abstract");
@@ -3028,35 +3010,33 @@ print_ada_declaration (pretty_printer *b
 
       pp_string (buffer, " is ");
 
-      /* Check whether we have an Ada interface compatible class.  */
+      /* Check whether we have an Ada interface compatible class.
+	 That is only have a vtable non-static data member and no
+	 non-abstract methods.  */
       if (cpp_check
-	  && RECORD_OR_UNION_TYPE_P (TREE_TYPE (t))
-	  && TYPE_METHODS (TREE_TYPE (t)))
+	  && RECORD_OR_UNION_TYPE_P (TREE_TYPE (t)))
 	{
-	  int num_fields = 0;
-	  tree tmp;
+	  is_interface = -1;
 
 	  /* Check that there are no fields other than the virtual table.  */
-	  for (tmp = TYPE_FIELDS (TREE_TYPE (t)); tmp; tmp = TREE_CHAIN (tmp))
+	  for (tree fld = TYPE_FIELDS (TREE_TYPE (t));
+	       fld; fld = TREE_CHAIN (fld))
 	    {
-	      if (TREE_CODE (tmp) == TYPE_DECL)
-		continue;
-	      num_fields++;
-	    }
-
-	  if (num_fields == 1)
-	    is_interface = 1;
-
-	  /* Also check that there are only pure virtual methods.  Since the
-	     class is empty, we can skip implicit constructors/destructors.  */
-	  for (tmp = TYPE_METHODS (TREE_TYPE (t)); tmp; tmp = TREE_CHAIN (tmp))
-	    {
-	      if (DECL_ARTIFICIAL (tmp))
-		continue;
-	      if (cpp_check (tmp, IS_ABSTRACT))
-		is_abstract_record = 1;
-	      else
-		is_interface = 0;
+	      if (TREE_CODE (fld) == FIELD_DECL)
+		{
+		  if (is_interface < 0 && DECL_VIRTUAL_P (fld))
+		    is_interface = 1;
+		  else
+		    is_interface = 0;
+		}
+	      else if (TREE_CODE (TREE_TYPE (fld)) == METHOD_TYPE
+		       && !DECL_ARTIFICIAL (fld))
+		{
+		  if (cpp_check (fld, IS_ABSTRACT))
+		    is_abstract_record = 1;
+		  else
+		    is_interface = 0;
+		}
 	    }
 	}
 

Re: [PATCH] Kill TYPE_METHODS c++ 5/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

This is the biggest chunk, in the C++ FE.

The only quirk is a hopefully short-lived additional check when looking up a 
non-function member in an incomplete type.  We iterate over TYPE_FIELDS, and 
should now ignore things that are functions -- because we'll find them as 
overloads when looking at the method-vector.  That's a subsequent cleanup.

nathan
-- 
Nathan Sidwell

[-- Attachment #2: method-cp.diff --]
[-- Type: text/plain, Size: 29818 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/cp/
	* class.c (maybe_warn_about_overly_private_class,
	finish_struct_methods, one_inheriting_sig, count_fields,
	add_fields_to_record_type, check_field_decls, check_methods,
	clone_function_decl, set_method_tm_attributes,
	finalize_literal_type_property, check_bases_and_members,
	create_vtable_ptr, determine_key_method,
	unreverse_member_declarations, finish_struct,
	add_vcall_offset_vtbl_entries_1): Member fns are on TYPE_FIELDS.
	* decl.c (fixup_anonymous_aggr): Likewise.
	* decl2.c (reset_type_linkage_2): Likewise.
	* method.c (after_nsdmi_defaulted_late_checks,
	lazily_declare_fn): Likewise.
	* optimize.c (maybe_thunk_body, maybe_clone_body): Likewise.
	* pt.c (instantiate_class_template_1, tsubst_expr,
	do_type_instantiation, instantiate_pending_templates): Likewise.
	* search.c (lookup_field_1): Likewise.
	* semantics.c (finish_member_declaration,
	finish_omp_declare_simd_methods): Likewise.

	gcc/testsuite/
	* g++.dg/ext/anon-struct6.C: Adjust diag.
	* g++.old-deja/g++.other/anon4.C: Adjust diag.

Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 250160)
+++ gcc/cp/class.c	(working copy)
@@ -2149,7 +2149,6 @@ maybe_warn_about_overly_private_class (t
 {
   int has_member_fn = 0;
   int has_nonprivate_method = 0;
-  tree fn;
 
   if (!warn_ctor_dtor_privacy
       /* If the class has friends, those entities might create and
@@ -2179,26 +2178,26 @@ maybe_warn_about_overly_private_class (t
      functions are private.  (Since there are no friends or
      non-private statics, we can't ever call any of the private member
      functions.)  */
-  for (fn = TYPE_METHODS (t); fn; fn = DECL_CHAIN (fn))
-    /* We're not interested in compiler-generated methods; they don't
-       provide any way to call private members.  */
-    if (!DECL_ARTIFICIAL (fn))
+  for (tree fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
+    if (!DECL_DECLARES_FUNCTION_P (fn))
+      /* Not a function.  */;
+    else if (DECL_ARTIFICIAL (fn))
+      /* We're not interested in compiler-generated methods; they don't
+	 provide any way to call private members.  */;
+    else if (!TREE_PRIVATE (fn))
       {
-	if (!TREE_PRIVATE (fn))
-	  {
-	    if (DECL_STATIC_FUNCTION_P (fn))
-	      /* A non-private static member function is just like a
-		 friend; it can create and invoke private member
-		 functions, and be accessed without a class
-		 instance.  */
-	      return;
+	if (DECL_STATIC_FUNCTION_P (fn))
+	  /* A non-private static member function is just like a
+	     friend; it can create and invoke private member
+	     functions, and be accessed without a class
+	     instance.  */
+	  return;
 
-	    has_nonprivate_method = 1;
-	    /* Keep searching for a static member function.  */
-	  }
-	else if (!DECL_CONSTRUCTOR_P (fn) && !DECL_DESTRUCTOR_P (fn))
-	  has_member_fn = 1;
+	has_nonprivate_method = 1;
+	/* Keep searching for a static member function.  */
       }
+    else if (!DECL_CONSTRUCTOR_P (fn) && !DECL_DESTRUCTOR_P (fn))
+      has_member_fn = 1;
 
   if (!has_nonprivate_method && has_member_fn)
     {
@@ -2228,14 +2227,14 @@ maybe_warn_about_overly_private_class (t
   /* Even if some of the member functions are non-private, the class
      won't be useful for much if all the constructors or destructors
      are private: such an object can never be created or destroyed.  */
-  fn = CLASSTYPE_DESTRUCTOR (t);
-  if (fn && TREE_PRIVATE (fn))
-    {
-      warning (OPT_Wctor_dtor_privacy,
-	       "%q#T only defines a private destructor and has no friends",
-	       t);
-      return;
-    }
+  if (tree dtor = CLASSTYPE_DESTRUCTOR (t))
+    if (TREE_PRIVATE (dtor))
+      {
+	warning (OPT_Wctor_dtor_privacy,
+		 "%q#T only defines a private destructor and has no friends",
+		 t);
+	return;
+      }
 
   /* Warn about classes that have private constructors and no friends.  */
   if (TYPE_HAS_USER_CONSTRUCTOR (t)
@@ -2367,7 +2366,6 @@ resort_type_method_vec (void* obj,
 static void
 finish_struct_methods (tree t)
 {
-  tree fn_fields;
   vec<tree, va_gc> *method_vec;
   int slot, len;
 
@@ -2378,9 +2376,9 @@ finish_struct_methods (tree t)
   len = method_vec->length ();
 
   /* Clear DECL_IN_AGGR_P for all functions.  */
-  for (fn_fields = TYPE_METHODS (t); fn_fields;
-       fn_fields = DECL_CHAIN (fn_fields))
-    DECL_IN_AGGR_P (fn_fields) = 0;
+  for (tree fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
+    if (DECL_DECLARES_FUNCTION_P (fn))
+      DECL_IN_AGGR_P (fn) = false;
 
   /* Issue warnings about private constructors and such.  If there are
      no methods, then some public defaults are generated.  */
@@ -2388,6 +2386,7 @@ finish_struct_methods (tree t)
 
   /* The type conversion ops have to live at the front of the vec, so we
      can't sort them.  */
+  tree fn_fields;
   for (slot = CLASSTYPE_FIRST_CONVERSION_SLOT;
        method_vec->iterate (slot, &fn_fields);
        ++slot)
@@ -3299,6 +3298,8 @@ declare_virt_assop_and_dtor (tree t)
 static void
 one_inheriting_sig (tree t, tree ctor, tree *parms, int nparms)
 {
+  gcc_assert (TYPE_MAIN_VARIANT (t) == t);
+
   /* We don't declare an inheriting ctor that would be a default,
      copy or move ctor for derived or base.  */
   if (nparms == 0)
@@ -3316,11 +3317,11 @@ one_inheriting_sig (tree t, tree ctor, t
     parmlist = tree_cons (NULL_TREE, parms[i], parmlist);
   tree fn = implicitly_declare_fn (sfk_inheriting_constructor,
 				   t, false, ctor, parmlist);
-  gcc_assert (TYPE_MAIN_VARIANT (t) == t);
+
   if (add_method (t, fn, false))
     {
-      DECL_CHAIN (fn) = TYPE_METHODS (t);
-      TYPE_METHODS (t) = fn;
+      DECL_CHAIN (fn) = TYPE_FIELDS (t);
+      TYPE_FIELDS (t) = fn;
     }
 }
 
@@ -3459,7 +3460,9 @@ count_fields (tree fields)
   int n_fields = 0;
   for (x = fields; x; x = DECL_CHAIN (x))
     {
-      if (TREE_CODE (x) == FIELD_DECL && ANON_AGGR_TYPE_P (TREE_TYPE (x)))
+      if (DECL_DECLARES_FUNCTION_P (x))
+	/* Functions are dealt with separately.  */;
+      else if (TREE_CODE (x) == FIELD_DECL && ANON_AGGR_TYPE_P (TREE_TYPE (x)))
 	n_fields += count_fields (TYPE_FIELDS (TREE_TYPE (x)));
       else
 	n_fields += 1;
@@ -3477,7 +3480,9 @@ add_fields_to_record_type (tree fields,
   tree x;
   for (x = fields; x; x = DECL_CHAIN (x))
     {
-      if (TREE_CODE (x) == FIELD_DECL && ANON_AGGR_TYPE_P (TREE_TYPE (x)))
+      if (DECL_DECLARES_FUNCTION_P (x))
+	/* Functions are handled separately.  */;
+      else if (TREE_CODE (x) == FIELD_DECL && ANON_AGGR_TYPE_P (TREE_TYPE (x)))
 	idx = add_fields_to_record_type (TYPE_FIELDS (TREE_TYPE (x)), field_vec, idx);
       else
 	field_vec->elts[idx++] = x;
@@ -3734,6 +3739,10 @@ check_field_decls (tree t, tree *access_
 	  || TREE_CODE (x) == TEMPLATE_DECL)
 	continue;
 
+      if (TREE_CODE (x) == FUNCTION_DECL)
+	/* FIXME: We should fold in the checking from check_methods.  */
+	continue;
+
       /* If we've gotten this far, it's a data member, possibly static,
 	 or an enumerator.  */
       if (TREE_CODE (x) != CONST_DECL)
@@ -4658,39 +4667,42 @@ build_base_fields (record_layout_info rl
     }
 }
 
-/* Go through the TYPE_METHODS of T issuing any appropriate
+/* Go through the TYPE_FIELDS of T issuing any appropriate
    diagnostics, figuring out which methods override which other
    methods, and so forth.  */
 
 static void
 check_methods (tree t)
 {
-  tree x;
+  for (tree x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
+    if (DECL_DECLARES_FUNCTION_P (x))
+      {
+	check_for_override (x, t);
 
-  for (x = TYPE_METHODS (t); x; x = DECL_CHAIN (x))
-    {
-      check_for_override (x, t);
-      if (DECL_PURE_VIRTUAL_P (x) && (TREE_CODE (x) != FUNCTION_DECL || ! DECL_VINDEX (x)))
-	error ("initializer specified for non-virtual method %q+D", x);
-      /* The name of the field is the original field name
-	 Save this in auxiliary field for later overloading.  */
-      if (TREE_CODE (x) == FUNCTION_DECL && DECL_VINDEX (x))
-	{
-	  TYPE_POLYMORPHIC_P (t) = 1;
-	  if (DECL_PURE_VIRTUAL_P (x))
-	    vec_safe_push (CLASSTYPE_PURE_VIRTUALS (t), x);
-	}
-      /* All user-provided destructors are non-trivial.
-         Constructors and assignment ops are handled in
-	 grok_special_member_properties.  */
-      if (DECL_DESTRUCTOR_P (x) && user_provided_p (x))
-	TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t) = 1;
-      if (!DECL_VIRTUAL_P (x)
-	  && lookup_attribute ("transaction_safe_dynamic", DECL_ATTRIBUTES (x)))
-	error_at (DECL_SOURCE_LOCATION (x),
-		  "%<transaction_safe_dynamic%> may only be specified for "
-		  "a virtual function");
-    }
+	if (DECL_PURE_VIRTUAL_P (x)
+	    && (TREE_CODE (x) != FUNCTION_DECL || ! DECL_VINDEX (x)))
+	  error ("initializer specified for non-virtual method %q+D", x);
+	/* The name of the field is the original field name
+	   Save this in auxiliary field for later overloading.  */
+	if (TREE_CODE (x) == FUNCTION_DECL && DECL_VINDEX (x))
+	  {
+	    TYPE_POLYMORPHIC_P (t) = 1;
+	    if (DECL_PURE_VIRTUAL_P (x))
+	      vec_safe_push (CLASSTYPE_PURE_VIRTUALS (t), x);
+	  }
+
+	/* All user-provided destructors are non-trivial.
+	   Constructors and assignment ops are handled in
+	   grok_special_member_properties.  */
+	if (DECL_DESTRUCTOR_P (x) && user_provided_p (x))
+	  TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t) = 1;
+	if (!DECL_VIRTUAL_P (x)
+	    && lookup_attribute ("transaction_safe_dynamic",
+				 DECL_ATTRIBUTES (x)))
+	  error_at (DECL_SOURCE_LOCATION (x),
+		    "%<transaction_safe_dynamic%> may only be specified for "
+		    "a virtual function");
+      }
 }
 
 /* FN is a constructor or destructor.  Clone the declaration to create
@@ -4896,7 +4908,7 @@ clone_function_decl (tree fn, bool updat
       /* For each destructor, we need three variants: an in-charge
 	 version, a not-in-charge version, and an in-charge deleting
 	 version.  We clone the deleting version first because that
-	 means it will go second on the TYPE_METHODS list -- and that
+	 means it will go second on the TYPE_FIELDS list -- and that
 	 corresponds to the correct layout order in the virtual
 	 function table.
 
@@ -5168,11 +5180,10 @@ set_method_tm_attributes (tree t)
 
   /* Any method that does not yet have a tm attribute inherits
      the one from the class.  */
-  for (fndecl = TYPE_METHODS (t); fndecl; fndecl = TREE_CHAIN (fndecl))
-    {
-      if (!find_tm_attribute (TYPE_ATTRIBUTES (TREE_TYPE (fndecl))))
-	apply_tm_attr (fndecl, class_tm_attr);
-    }
+  for (fndecl = TYPE_FIELDS (t); fndecl; fndecl = DECL_CHAIN (fndecl))
+    if (DECL_DECLARES_FUNCTION_P (fndecl)
+	&& !find_tm_attribute (TYPE_ATTRIBUTES (TREE_TYPE (fndecl))))
+      apply_tm_attr (fndecl, class_tm_attr);
 }
 
 /* Returns true if FN is a default constructor.  */
@@ -5705,9 +5716,9 @@ finalize_literal_type_property (tree t)
   /* C++14 DR 1684 removed this restriction.  */
   if (cxx_dialect < cxx14
       && !CLASSTYPE_LITERAL_P (t) && !LAMBDA_TYPE_P (t))
-    for (fn = TYPE_METHODS (t); fn; fn = DECL_CHAIN (fn))
-      if (DECL_DECLARED_CONSTEXPR_P (fn)
-	  && TREE_CODE (fn) != TEMPLATE_DECL
+    for (fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
+      if (TREE_CODE (fn) == FUNCTION_DECL
+	  && DECL_DECLARED_CONSTEXPR_P (fn)
 	  && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
 	  && !DECL_CONSTRUCTOR_P (fn))
 	{
@@ -5969,8 +5980,10 @@ check_bases_and_members (tree t)
 
   /* Check defaulted declarations here so we have cant_have_const_ctor
      and don't need to worry about clones.  */
-  for (fn = TYPE_METHODS (t); fn; fn = DECL_CHAIN (fn))
-    if (!DECL_ARTIFICIAL (fn) && DECL_DEFAULTED_IN_CLASS_P (fn))
+  for (fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
+    if (DECL_DECLARES_FUNCTION_P (fn)
+	&& !DECL_ARTIFICIAL (fn)
+	&& DECL_DEFAULTED_IN_CLASS_P (fn))
       {
 	int copy = copy_fn_p (fn);
 	if (copy > 0)
@@ -6029,7 +6042,7 @@ create_vtable_ptr (tree t, tree* virtual
   tree fn;
 
   /* Collect the virtual functions declared in T.  */
-  for (fn = TYPE_METHODS (t); fn; fn = DECL_CHAIN (fn))
+  for (fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
     if (TREE_CODE (fn) == FUNCTION_DECL
 	&& DECL_VINDEX (fn) && !DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (fn)
 	&& TREE_CODE (DECL_VINDEX (fn)) != INTEGER_CST)
@@ -6688,8 +6701,7 @@ determine_key_method (tree type)
      inline at the point of class definition.  On some targets the
      key function may not be inline; those targets should not call
      this function until the end of the translation unit.  */
-  for (method = TYPE_METHODS (type); method != NULL_TREE;
-       method = DECL_CHAIN (method))
+  for (method = TYPE_FIELDS (type); method; method = DECL_CHAIN (method))
     if (TREE_CODE (method) == FUNCTION_DECL
 	&& DECL_VINDEX (method) != NULL_TREE
 	&& ! DECL_DECLARED_INLINE_P (method)
@@ -7381,11 +7393,11 @@ unreverse_member_declarations (tree t)
 
   /* The following lists are all in reverse order.  Put them in
      declaration order now.  */
-  TYPE_METHODS (t) = nreverse (TYPE_METHODS (t));
   CLASSTYPE_DECL_LIST (t) = nreverse (CLASSTYPE_DECL_LIST (t));
 
-  /* Actually, for the TYPE_FIELDS, only the non TYPE_DECLs are in
-     reverse order, so we can't just use nreverse.  */
+  /* For the TYPE_FIELDS, only the non TYPE_DECLs are in reverse
+     order, so we can't just use nreverse.  Due to stat_hack
+     chicanery in finish_member_declarations.  */
   prev = NULL_TREE;
   for (x = TYPE_FIELDS (t);
        x && TREE_CODE (x) != TYPE_DECL;
@@ -7395,6 +7407,7 @@ unreverse_member_declarations (tree t)
       DECL_CHAIN (x) = prev;
       prev = x;
     }
+
   if (prev)
     {
       DECL_CHAIN (TYPE_FIELDS (t)) = x;
@@ -7435,8 +7448,8 @@ finish_struct (tree t, tree attributes)
 	 CLASSTYPE_PURE_VIRTUALS contains the list of the inline friends
 	 (see CLASSTYPE_INLINE_FRIENDS) so we need to clear it.  */
       CLASSTYPE_PURE_VIRTUALS (t) = NULL;
-      for (x = TYPE_METHODS (t); x; x = DECL_CHAIN (x))
-	if (DECL_PURE_VIRTUAL_P (x))
+      for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
+	if (TREE_CODE (x) == FUNCTION_DECL && DECL_PURE_VIRTUAL_P (x))
 	  vec_safe_push (CLASSTYPE_PURE_VIRTUALS (t), x);
       complete_vars (t);
       /* We need to add the target functions to the CLASSTYPE_METHOD_VEC if
@@ -7461,7 +7474,6 @@ finish_struct (tree t, tree attributes)
 	  TYPE_SIZE (x) = TYPE_SIZE (t);
 	  TYPE_SIZE_UNIT (x) = TYPE_SIZE_UNIT (t);
 	  TYPE_FIELDS (x) = TYPE_FIELDS (t);
-	  TYPE_METHODS (x) = TYPE_METHODS (t);
 	}
     }
   else
@@ -9967,7 +9979,7 @@ add_vcall_offset_vtbl_entries_1 (tree bi
 
   /* The ABI requires that the methods be processed in declaration
      order.  */
-  for (orig_fn = TYPE_METHODS (BINFO_TYPE (binfo));
+  for (orig_fn = TYPE_FIELDS (BINFO_TYPE (binfo));
        orig_fn;
        orig_fn = DECL_CHAIN (orig_fn))
     if (TREE_CODE (orig_fn) == FUNCTION_DECL && DECL_VINDEX (orig_fn))
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 250160)
+++ gcc/cp/decl.c	(working copy)
@@ -4549,8 +4549,6 @@ push_throw_library_fn (tree name, tree t
 void
 fixup_anonymous_aggr (tree t)
 {
-  tree *q;
-
   /* Wipe out memory of synthesized methods.  */
   TYPE_HAS_USER_CONSTRUCTOR (t) = 0;
   TYPE_HAS_DEFAULT_CONSTRUCTOR (t) = 0;
@@ -4559,29 +4557,12 @@ fixup_anonymous_aggr (tree t)
   TYPE_HAS_COPY_ASSIGN (t) = 0;
   TYPE_HAS_CONST_COPY_ASSIGN (t) = 0;
 
-  /* Splice the implicitly generated functions out of the TYPE_METHODS
-     list.  */
-  q = &TYPE_METHODS (t);
-  while (*q)
-    {
-      if (DECL_ARTIFICIAL (*q))
-	*q = TREE_CHAIN (*q);
-      else
-	q = &DECL_CHAIN (*q);
-    }
-
-  /* ISO C++ 9.5.3.  Anonymous unions may not have function members.  */
-  if (TYPE_METHODS (t))
-    {
-      tree decl = TYPE_MAIN_DECL (t);
-
-      if (TREE_CODE (t) != UNION_TYPE)
-	error_at (DECL_SOURCE_LOCATION (decl), 
-		  "an anonymous struct cannot have function members");
-      else
-	error_at (DECL_SOURCE_LOCATION (decl),
-		  "an anonymous union cannot have function members");
-    }
+  /* Splice the implicitly generated functions out of TYPE_FIELDS.  */
+  for (tree probe, *prev_p = &TYPE_FIELDS (t); (probe = *prev_p);)
+    if (TREE_CODE (probe) == FUNCTION_DECL && DECL_ARTIFICIAL (probe))
+      *prev_p = DECL_CHAIN (probe);
+    else
+      prev_p = &DECL_CHAIN (probe);
 
   /* Anonymous aggregates cannot have fields with ctors, dtors or complex
      assignment operators (because they cannot have these methods themselves).
Index: gcc/cp/decl2.c
===================================================================
--- gcc/cp/decl2.c	(revision 250160)
+++ gcc/cp/decl2.c	(working copy)
@@ -2592,6 +2592,7 @@ reset_decl_linkage (tree decl)
   determine_visibility (decl);
   tentative_decl_linkage (decl);
 }
+
 static void
 reset_type_linkage_2 (tree type)
 {
@@ -2615,18 +2616,14 @@ reset_type_linkage_2 (tree type)
       for (tree m = TYPE_FIELDS (type); m; m = DECL_CHAIN (m))
 	{
 	  tree mem = STRIP_TEMPLATE (m);
-	  if (VAR_P (mem))
+	  if (TREE_CODE (mem) == VAR_DECL || TREE_CODE (mem) == FUNCTION_DECL)
 	    reset_decl_linkage (mem);
 	}
-      for (tree m = TYPE_METHODS (type); m; m = DECL_CHAIN (m))
-	{
-	  tree mem = STRIP_TEMPLATE (m);
-	  reset_decl_linkage (mem);
-	}
       binding_table_foreach (CLASSTYPE_NESTED_UTDS (type),
 			     bt_reset_linkage_2, NULL);
     }
 }
+
 static void
 bt_reset_linkage_2 (binding_entry b, void */*data*/)
 {
@@ -4997,19 +4994,13 @@ mark_used (tree decl, tsubst_flags_t com
   if (TREE_CODE (decl) == FUNCTION_DECL
       && DECL_DELETED_FN (decl))
     {
-      if (DECL_ARTIFICIAL (decl))
-	{
-	  if (DECL_OVERLOADED_OPERATOR_P (decl) == TYPE_EXPR
-	      && LAMBDA_TYPE_P (DECL_CONTEXT (decl)))
-	    {
-	      /* We mark a lambda conversion op as deleted if we can't
-		 generate it properly; see maybe_add_lambda_conv_op.  */
-	      sorry ("converting lambda which uses %<...%> to "
-		     "function pointer");
-	      return false;
-	    }
-	}
-      if (complain & tf_error)
+      if (DECL_ARTIFICIAL (decl)
+	  && DECL_OVERLOADED_OPERATOR_P (decl) == TYPE_EXPR
+	  && LAMBDA_TYPE_P (DECL_CONTEXT (decl)))
+	/* We mark a lambda conversion op as deleted if we can't
+	   generate it properly; see maybe_add_lambda_conv_op.  */
+	sorry ("converting lambda which uses %<...%> to function pointer");
+      else if (complain & tf_error)
 	{
 	  error ("use of deleted function %qD", decl);
 	  if (!maybe_explain_implicit_delete (decl))
Index: gcc/cp/method.c
===================================================================
--- gcc/cp/method.c	(revision 250160)
+++ gcc/cp/method.c	(working copy)
@@ -2248,8 +2248,10 @@ after_nsdmi_defaulted_late_checks (tree
     return;
   if (t == error_mark_node)
     return;
-  for (tree fn = TYPE_METHODS (t); fn; fn = DECL_CHAIN (fn))
-    if (!DECL_ARTIFICIAL (fn) && DECL_DEFAULTED_IN_CLASS_P (fn))
+  for (tree fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
+    if (!DECL_ARTIFICIAL (fn)
+	&& DECL_DECLARES_FUNCTION_P (fn)
+	&& DECL_DEFAULTED_IN_CLASS_P (fn))
       {
 	tree fn_spec = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn));
 	if (UNEVALUATED_NOEXCEPT_SPEC_P (fn_spec))
@@ -2383,20 +2385,25 @@ lazily_declare_fn (special_function_kind
       || sfk == sfk_move_assignment
       || sfk == sfk_copy_assignment)
     check_for_override (fn, type);
+
   /* Add it to CLASSTYPE_METHOD_VEC.  */
   bool added = add_method (type, fn, false);
   gcc_assert (added);
-  /* Add it to TYPE_METHODS.  */
+
+  /* Add it to TYPE_FIELDS.  */
   if (sfk == sfk_destructor
       && DECL_VIRTUAL_P (fn))
     /* The ABI requires that a virtual destructor go at the end of the
        vtable.  */
-    TYPE_METHODS (type) = chainon (TYPE_METHODS (type), fn);
+    TYPE_FIELDS (type) = chainon (TYPE_FIELDS (type), fn);
   else
     {
-      DECL_CHAIN (fn) = TYPE_METHODS (type);
-      TYPE_METHODS (type) = fn;
+      DECL_CHAIN (fn) = TYPE_FIELDS (type);
+      TYPE_FIELDS (type) = fn;
     }
+  /* Propagate TYPE_FIELDS.  */
+  fixup_type_variants (type);
+
   maybe_add_class_template_decl_list (type, fn, /*friend_p=*/0);
   if (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (fn)
       || DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (fn))
Index: gcc/cp/optimize.c
===================================================================
--- gcc/cp/optimize.c	(revision 250160)
+++ gcc/cp/optimize.c	(working copy)
@@ -326,7 +326,7 @@ maybe_thunk_body (tree fn, bool force)
     }
   args = XALLOCAVEC (tree, max_parms);
 
-  /* We know that any clones immediately follow FN in TYPE_METHODS.  */
+  /* We know that any clones immediately follow FN in TYPE_FIELDS.  */
   FOR_EACH_CLONE (clone, fn)
     {
       tree clone_parm;
@@ -447,7 +447,7 @@ maybe_clone_body (tree fn)
   if (!tree_versionable_function_p (fn))
     need_alias = true;
 
-  /* We know that any clones immediately follow FN in the TYPE_METHODS
+  /* We know that any clones immediately follow FN in the TYPE_FIELDS
      list.  */
   push_to_top_level ();
   for (idx = 0; idx < 3; idx++)
@@ -516,7 +516,7 @@ maybe_clone_body (tree fn)
   /* Emit the DWARF1 abstract instance.  */
   (*debug_hooks->deferred_inline_function) (fn);
 
-  /* We know that any clones immediately follow FN in the TYPE_METHODS list. */
+  /* We know that any clones immediately follow FN in the TYPE_FIELDS. */
   for (idx = 0; idx < 3; idx++)
     {
       tree parm;
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 250160)
+++ gcc/cp/pt.c	(working copy)
@@ -10550,7 +10550,6 @@ instantiate_class_template_1 (tree type)
 	    }
 	  else if (DECL_DECLARES_FUNCTION_P (t))
 	    {
-	      /* Build new TYPE_METHODS.  */
 	      tree r;
 
 	      if (TREE_CODE (t) == TEMPLATE_DECL)
@@ -16136,13 +16135,15 @@ tsubst_expr (tree t, tree args, tsubst_f
 	     instantiated along with their containing function.  And this
 	     way we don't have to deal with pushing out of one local class
 	     to instantiate a member of another local class.  */
-	  tree fn;
 	  /* Closures are handled by the LAMBDA_EXPR.  */
 	  gcc_assert (!LAMBDA_TYPE_P (TREE_TYPE (t)));
 	  complete_type (tmp);
-	  for (fn = TYPE_METHODS (tmp); fn; fn = DECL_CHAIN (fn))
-	    if (!DECL_ARTIFICIAL (fn))
-	      instantiate_decl (fn, /*defer_ok=*/false,
+	  for (tree fld = TYPE_FIELDS (tmp); fld; fld = DECL_CHAIN (fld))
+	    if ((VAR_P (fld)
+		 || (TREE_CODE (fld) == FUNCTION_DECL
+		     && !DECL_ARTIFICIAL (fld)))
+		&& DECL_TEMPLATE_INSTANTIATION (fld))
+	      instantiate_decl (fld, /*defer_ok=*/false,
 				/*expl_inst_class=*/false);
 	}
       break;
@@ -22132,18 +22133,6 @@ bt_instantiate_type_proc (binding_entry
     do_type_instantiation (TYPE_MAIN_DECL (entry->type), storage, 0);
 }
 
-/* Called from do_type_instantiation to instantiate a member
-   (a member function or a static member variable) of an
-   explicitly instantiated class template.  */
-static void
-instantiate_class_member (tree decl, int extern_p)
-{
-  mark_decl_instantiated (decl, extern_p);
-  if (! extern_p)
-    instantiate_decl (decl, /*defer_ok=*/true,
-		      /*expl_inst_class_mem_p=*/true);
-}
-
 /* Perform an explicit instantiation of template class T.  STORAGE, if
    non-null, is the RID for extern, inline or static.  COMPLAIN is
    nonzero if this is called from the parser, zero if called recursively,
@@ -22253,12 +22242,9 @@ do_type_instantiation (tree t, tree stor
   if (nomem_p)
     return;
 
-  {
-    tree tmp;
-
-    /* In contrast to implicit instantiation, where only the
-       declarations, and not the definitions, of members are
-       instantiated, we have here:
+  /* In contrast to implicit instantiation, where only the
+     declarations, and not the definitions, of members are
+     instantiated, we have here:
 
 	 [temp.explicit]
 
@@ -22267,27 +22253,28 @@ do_type_instantiation (tree t, tree stor
 	 previously explicitly specialized in the translation unit
 	 containing the explicit instantiation.
 
-       Of course, we can't instantiate member template classes, since
-       we don't have any arguments for them.  Note that the standard
-       is unclear on whether the instantiation of the members are
-       *explicit* instantiations or not.  However, the most natural
-       interpretation is that it should be an explicit instantiation.  */
-
-    if (! static_p)
-      for (tmp = TYPE_METHODS (t); tmp; tmp = DECL_CHAIN (tmp))
-	if (TREE_CODE (tmp) == FUNCTION_DECL
-	    && DECL_TEMPLATE_INSTANTIATION (tmp)
-	    && user_provided_p (tmp))
-	  instantiate_class_member (tmp, extern_p);
-
-    for (tmp = TYPE_FIELDS (t); tmp; tmp = DECL_CHAIN (tmp))
-      if (VAR_P (tmp) && DECL_TEMPLATE_INSTANTIATION (tmp))
-	instantiate_class_member (tmp, extern_p);
-
-    if (CLASSTYPE_NESTED_UTDS (t))
-      binding_table_foreach (CLASSTYPE_NESTED_UTDS (t),
-			     bt_instantiate_type_proc, &storage);
-  }
+     Of course, we can't instantiate member template classes, since we
+     don't have any arguments for them.  Note that the standard is
+     unclear on whether the instantiation of the members are
+     *explicit* instantiations or not.  However, the most natural
+     interpretation is that it should be an explicit
+     instantiation.  */
+  for (tree fld = TYPE_FIELDS (t); fld; fld = DECL_CHAIN (fld))
+    if ((VAR_P (fld)
+	 || (TREE_CODE (fld) == FUNCTION_DECL
+	     && !static_p
+	     && user_provided_p (fld)))
+	&& DECL_TEMPLATE_INSTANTIATION (fld))
+      {
+	mark_decl_instantiated (fld, extern_p);
+	if (! extern_p)
+	  instantiate_decl (fld, /*defer_ok=*/true,
+			    /*expl_inst_class_mem_p=*/true);
+      }
+
+  if (CLASSTYPE_NESTED_UTDS (t))
+    binding_table_foreach (CLASSTYPE_NESTED_UTDS (t),
+			   bt_instantiate_type_proc, &storage);
 }
 
 /* Given a function DECL, which is a specialization of TMPL, modify
@@ -23079,19 +23066,20 @@ instantiate_pending_templates (int retri
 
 	  if (TYPE_P (instantiation))
 	    {
-	      tree fn;
-
 	      if (!COMPLETE_TYPE_P (instantiation))
 		{
 		  instantiate_class_template (instantiation);
 		  if (CLASSTYPE_TEMPLATE_INSTANTIATION (instantiation))
-		    for (fn = TYPE_METHODS (instantiation);
-			 fn;
-			 fn = TREE_CHAIN (fn))
-		      if (! DECL_ARTIFICIAL (fn))
-			instantiate_decl (fn,
+		    for (tree fld = TYPE_FIELDS (instantiation);
+			 fld; fld = TREE_CHAIN (fld))
+		      if ((VAR_P (fld)
+			   || (TREE_CODE (fld) == FUNCTION_DECL
+			       && !DECL_ARTIFICIAL (fld)))
+			  && DECL_TEMPLATE_INSTANTIATION (fld))
+			instantiate_decl (fld,
 					  /*defer_ok=*/false,
 					  /*expl_inst_class_mem_p=*/false);
+
 		  if (COMPLETE_TYPE_P (instantiation))
 		    reconsider = 1;
 		}
Index: gcc/cp/search.c
===================================================================
--- gcc/cp/search.c	(revision 250160)
+++ gcc/cp/search.c	(working copy)
@@ -444,6 +444,10 @@ lookup_field_1 (tree type, tree name, bo
     {
       tree decl = field;
 
+      if (DECL_DECLARES_FUNCTION_P (decl))
+	/* Functions are kep separately, at the moment.  */
+	continue;
+
       if (GATHER_STATISTICS)
 	n_fields_searched++;
 
Index: gcc/cp/semantics.c
===================================================================
--- gcc/cp/semantics.c	(revision 250160)
+++ gcc/cp/semantics.c	(working copy)
@@ -3037,9 +3037,9 @@ finish_member_declaration (tree decl)
   if (DECL_LANG_SPECIFIC (decl) && DECL_LANGUAGE (decl) == lang_c)
     SET_DECL_LANGUAGE (decl, lang_cplusplus);
 
-  /* Put functions on the TYPE_METHODS list and everything else on the
-     TYPE_FIELDS list.  Note that these are built up in reverse order.
-     We reverse them (to obtain declaration order) in finish_struct.  */
+  /* Put the decl on the TYPE_FIELDS list.  Note that this is built up
+     in reverse order.  We reverse it (to obtain declaration order) in
+     finish_struct.  */
   if (DECL_DECLARES_FUNCTION_P (decl))
     {
       /* We also need to add this function to the
@@ -3047,8 +3047,8 @@ finish_member_declaration (tree decl)
       if (add_method (current_class_type, decl, false))
 	{
 	  gcc_assert (TYPE_MAIN_VARIANT (current_class_type) == current_class_type);
-	  DECL_CHAIN (decl) = TYPE_METHODS (current_class_type);
-	  TYPE_METHODS (current_class_type) = decl;
+	  DECL_CHAIN (decl) = TYPE_FIELDS (current_class_type);
+	  TYPE_FIELDS (current_class_type) = decl;
 
 	  maybe_add_class_template_decl_list (current_class_type, decl,
 					      /*friend_p=*/0);
@@ -5794,7 +5794,7 @@ finish_omp_declare_simd_methods (tree t)
   if (processing_template_decl)
     return;
 
-  for (tree x = TYPE_METHODS (t); x; x = DECL_CHAIN (x))
+  for (tree x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     {
       if (TREE_CODE (TREE_TYPE (x)) != METHOD_TYPE)
 	continue;
Index: gcc/testsuite/g++.dg/ext/anon-struct6.C
===================================================================
--- gcc/testsuite/g++.dg/ext/anon-struct6.C	(revision 250160)
+++ gcc/testsuite/g++.dg/ext/anon-struct6.C	(working copy)
@@ -3,8 +3,8 @@
 struct A
 {
   struct
-  {  // { dg-error "anonymous struct cannot have function members" }
+  {
     struct { static int i; }; // { dg-error "prohibits anonymous structs|non-static data members|unnamed class" }
-    void foo() { i; }
+    void foo() { i; } // { dg-error "can only have non-static data" }
   }; // { dg-error "prohibits anonymous structs" }
 };
Index: gcc/testsuite/g++.old-deja/g++.other/anon4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/anon4.C	(revision 250160)
+++ gcc/testsuite/g++.old-deja/g++.other/anon4.C	(working copy)
@@ -10,7 +10,7 @@
 struct A
 {
   union
-  {  // { dg-error "" } anon union cannot have member fns
-    void bad();
+  {
+    void bad(); // { dg-error "can only have non-static data" }
   };
 };

Re: [PATCH] Kill TYPE_METHODS libcc1 6/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 121 bytes --]

This is the libcc1 change.  When creating a clone, it needs to fiddle with 
TYPE_FIELDS now.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-libcc1.diff --]
[-- Type: text/plain, Size: 1238 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	libcc1/
	* libcp1plugin.cc (plugin_build_decl): Member fns are on TYPE_FIELDS.

Index: libcc1/libcp1plugin.cc
===================================================================
--- libcc1/libcp1plugin.cc	(revision 250160)
+++ libcc1/libcp1plugin.cc	(working copy)
@@ -1556,7 +1556,7 @@ plugin_build_decl (cc1_plugin::connectio
 
   if ((ctor || dtor)
       /* Don't crash after a duplicate declaration of a cdtor.  */
-      && TYPE_METHODS (current_class_type) == decl)
+      && TYPE_FIELDS (current_class_type) == decl)
     {
       /* ctors and dtors clones are chained after DECL.
 	 However, we create the clones before TYPE_METHODS is
@@ -1568,9 +1568,9 @@ plugin_build_decl (cc1_plugin::connectio
       tree save = DECL_CHAIN (decl);
       DECL_CHAIN (decl) = NULL_TREE;
       clone_function_decl (decl, /*update_methods=*/true);
-      gcc_assert (TYPE_METHODS (current_class_type) == decl);
-      TYPE_METHODS (current_class_type)
-	= nreverse (TYPE_METHODS (current_class_type));
+      gcc_assert (TYPE_FIELDS (current_class_type) == decl);
+      TYPE_FIELDS (current_class_type)
+	= nreverse (TYPE_FIELDS (current_class_type));
       DECL_CHAIN (decl) = save;
     }
 

Re: [PATCH] Kill TYPE_METHODS misc 7/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

This bit removes knowledge of TYPE_METHODS from the various tree dumpers, 
allocators and deleters.

As mentioned before, the only reason we set TYPE_METHODS to error_mark_node when 
handing over to the middle end is for the RTL check about register allocation. 
But I'm deleting that check.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-misc.diff --]
[-- Type: text/plain, Size: 5806 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* tree.c (free_lang_data_in_type): Stitch out member functions and
	templates from TYPE_FIELDS.
	(build_distinct_type_copy, verify_type_variant,
	verify_type): Member fns are on TYPE_FIELDS.
	* tree-dump.c (dequeue_and_dump): No TYPE_METHODS.
	* tree-pretty-print.c (dump_generic_node): Likewise.

Index: gcc/tree-dump.c
===================================================================
--- gcc/tree-dump.c	(revision 250160)
+++ gcc/tree-dump.c	(working copy)
@@ -490,7 +490,6 @@ dequeue_and_dump (dump_info_p di)
 	dump_string_field (di, "tag", "union");
 
       dump_child ("flds", TYPE_FIELDS (t));
-      dump_child ("fncs", TYPE_METHODS (t));
       queue_and_dump_index (di, "binf", TYPE_BINFO (t),
 			    DUMP_BINFO);
       break;
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c	(revision 250160)
+++ gcc/tree-pretty-print.c	(working copy)
@@ -1860,22 +1860,9 @@ dump_generic_node (pretty_printer *pp, t
 	dump_decl_name (pp, node, flags);
       else if (TYPE_NAME (TREE_TYPE (node)) != node)
 	{
-	  if ((TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
-	       || TREE_CODE (TREE_TYPE (node)) == UNION_TYPE)
-	      && TYPE_METHODS (TREE_TYPE (node)))
-	    {
-	      /* The type is a c++ class: all structures have at least
-		 4 methods.  */
-	      pp_string (pp, "class ");
-	      dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
-	    }
-	  else
-	    {
-	      pp_string (pp,
-			 (TREE_CODE (TREE_TYPE (node)) == UNION_TYPE
+	  pp_string (pp, (TREE_CODE (TREE_TYPE (node)) == UNION_TYPE
 			  ? "union" : "struct "));
-	      dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
-	    }
+	  dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
 	}
       else
 	pp_string (pp, "<anon>");
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 250160)
+++ gcc/tree.c	(working copy)
@@ -5221,13 +5221,15 @@ free_lang_data_in_type (tree type)
       if (TYPE_VFIELD (type) && TREE_CODE (TYPE_VFIELD (type)) != FIELD_DECL)
         TYPE_VFIELD (type) = NULL_TREE;
 
-      /* Remove TYPE_METHODS list.  While it would be nice to keep it
- 	 to enable ODR warnings about different method lists, doing so
-	 seems to impractically increase size of LTO data streamed.
-	 Keep the information if TYPE_METHODS was non-NULL. This is used
-	 by function.c and pretty printers.  */
-      if (TYPE_METHODS (type))
-        TYPE_METHODS (type) = error_mark_node;
+      /* Splice out FUNCTION_DECLS and TEMPLATE_DECLS from
+	 TYPE_FIELDS.  So LTO doesn't grow.  */
+      for (tree probe, *prev= &TYPE_FIELDS (type); (probe = *prev); )
+	if (TREE_CODE (probe) == FUNCTION_DECL
+	    || TREE_CODE (probe) == TEMPLATE_DECL)
+	  *prev = probe;
+	else
+	  prev = &DECL_CHAIN (probe);
+
       if (TYPE_BINFO (type))
 	{
 	  free_lang_data_in_binfo (TYPE_BINFO (type));
@@ -5422,9 +5424,10 @@ free_lang_data_in_decl (tree decl)
 	 At this point, it is not needed anymore.  */
       DECL_SAVED_TREE (decl) = NULL_TREE;
 
-      /* Clear the abstract origin if it refers to a method.  Otherwise
-         dwarf2out.c will ICE as we clear TYPE_METHODS and thus the
-	 origin will not be output correctly.  */
+      /* Clear the abstract origin if it refers to a method.
+         Otherwise dwarf2out.c will ICE as we splice functions out of
+         TYPE_FIELDS and thus the origin will not be output
+         correctly.  */
       if (DECL_ABSTRACT_ORIGIN (decl)
 	  && DECL_CONTEXT (DECL_ABSTRACT_ORIGIN (decl))
 	  && RECORD_OR_UNION_TYPE_P
@@ -6692,12 +6695,6 @@ build_distinct_type_copy (tree type MEM_
   TYPE_MAIN_VARIANT (t) = t;
   TYPE_NEXT_VARIANT (t) = 0;
 
-  /* We do not record methods in type copies nor variants
-     so we do not need to keep them up to date when new method
-     is inserted.  */
-  if (RECORD_OR_UNION_TYPE_P (t))
-    TYPE_METHODS (t) = NULL_TREE;
-
   /* Note that it is now possible for TYPE_MIN_VALUE to be a value
      whose TREE_TYPE is not t.  This can also happen in the Ada
      frontend when using subtypes.  */
@@ -13423,8 +13420,6 @@ verify_type_variant (const_tree t, tree
      - aggregates may have new TYPE_FIELDS list that list variants of
        the main variant TYPE_FIELDS.
      - vector types may differ by TYPE_VECTOR_OPAQUE
-     - TYPE_METHODS is always NULL for variant types and maintained for
-       main variant only.
    */
 
   /* Convenience macro for matching individual fields.  */
@@ -13525,12 +13520,6 @@ verify_type_variant (const_tree t, tree
     }
   if (TREE_CODE (t) == METHOD_TYPE)
     verify_variant_match (TYPE_METHOD_BASETYPE);
-  if (RECORD_OR_UNION_TYPE_P (t) && TYPE_METHODS (t))
-    {
-      error ("type variant has TYPE_METHODS");
-      debug_tree (tv);
-      return false;
-    }
   if (TREE_CODE (t) == OFFSET_TYPE)
     verify_variant_match (TYPE_OFFSET_BASETYPE);
   if (TREE_CODE (t) == ARRAY_TYPE)
@@ -14042,14 +14031,6 @@ verify_type (const_tree t)
   /* Check various uses of TYPE_MAXVAL.  */
   if (RECORD_OR_UNION_TYPE_P (t))
     {
-      if (TYPE_METHODS (t) && TREE_CODE (TYPE_METHODS (t)) != FUNCTION_DECL
-	  && TREE_CODE (TYPE_METHODS (t)) != TEMPLATE_DECL
-	  && TYPE_METHODS (t) != error_mark_node)
-	{
-	  error ("TYPE_METHODS is not FUNCTION_DECL, TEMPLATE_DECL nor error_mark_node");
-	  debug_tree (TYPE_METHODS (t));
-	  error_found = true;
-	}
     }
   else if (TREE_CODE (t) == FUNCTION_TYPE || TREE_CODE (t) == METHOD_TYPE)
     {
@@ -14188,6 +14169,8 @@ verify_type (const_tree t)
 	    ;
 	  else if (TREE_CODE (fld) == USING_DECL)
 	    ;
+	  else if (TREE_CODE (fld) == FUNCTION_DECL)
+	    ;
 	  else
 	    {
 	      error ("Wrong tree in TYPE_FIELDS list");

Re: [PATCH] Kill TYPE_METHODS objc 8/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 280 bytes --]

This bit of the objective C runtime already knows that TYPE_FIELDS might contain 
non-FIELD_DECL things.  But it has the delightful assumption that the first 
thing is a FIELD_DECL, which apparently was fine until now.  Fixed by 
simplifying the loop.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-objc.diff --]
[-- Type: text/plain, Size: 2304 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/objc/
	* objc-runtime-shared-support.c (build_ivar_list_initializer):
	Don't presume first item is a FIELD_DECL.

Index: gcc/objc/objc-runtime-shared-support.c
===================================================================
--- gcc/objc/objc-runtime-shared-support.c	(revision 250160)
+++ gcc/objc/objc-runtime-shared-support.c	(working copy)
@@ -528,34 +528,32 @@ build_ivar_list_initializer (tree type,
 {
   vec<constructor_elt, va_gc> *inits = NULL;
 
-  do
-    {
-      vec<constructor_elt, va_gc> *ivar = NULL;
-      tree id;
+  for (; field_decl; field_decl = DECL_CHAIN (field_decl))
+    if (TREE_CODE (field_decl) == FIELD_DECL)
+      {
+	vec<constructor_elt, va_gc> *ivar = NULL;
+	tree id;
 
-      /* Set name.  */
-      if (DECL_NAME (field_decl))
-	CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE,
-				add_objc_string (DECL_NAME (field_decl),
-						 meth_var_names));
-      else
-	/* Unnamed bit-field ivar (yuck).  */
-	CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, build_int_cst (NULL_TREE, 0));
+	/* Set name.  */
+	if (DECL_NAME (field_decl))
+	  CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE,
+				  add_objc_string (DECL_NAME (field_decl),
+						   meth_var_names));
+	else
+	  /* Unnamed bit-field ivar (yuck).  */
+	  CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE,
+				  build_int_cst (NULL_TREE, 0));
 
-      /* Set type.  */
-      id = add_objc_string (encode_field_decl (field_decl),
-                            meth_var_types);
-      CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, id);
+	/* Set type.  */
+	id = add_objc_string (encode_field_decl (field_decl),
+			      meth_var_types);
+	CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, id);
 
-      /* Set offset.  */
-      CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, byte_position (field_decl));
-      CONSTRUCTOR_APPEND_ELT (inits, NULL_TREE,
-			      objc_build_constructor (type, ivar));
-      do
-	field_decl = DECL_CHAIN (field_decl);
-      while (field_decl && TREE_CODE (field_decl) != FIELD_DECL);
+	/* Set offset.  */
+	CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, byte_position (field_decl));
+	CONSTRUCTOR_APPEND_ELT (inits, NULL_TREE,
+				objc_build_constructor (type, ivar));
     }
-  while (field_decl);
 
   return objc_build_constructor (build_array_type (type, 0), inits);
 }

Re: [PATCH] Kill TYPE_METHODS 9/9

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

And finally, this patch kills the TYPE_METHODS macro.

As I mentioned earlier, I have other patches to repurpose the now-unused slot in 
records and unions.

nathan

-- 
Nathan Sidwell

[-- Attachment #2: method-ectomy.diff --]
[-- Type: text/plain, Size: 646 bytes --]

2017-07-14  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* tree.h (TYPE_METHODS): Delete.

Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 250160)
+++ gcc/tree.h	(working copy)
@@ -2110,8 +2110,6 @@ extern machine_mode element_mode (const_
   (FUNC_OR_METHOD_CHECK (NODE)->type_non_common.values)
 #define TYPE_VALUES_RAW(NODE) (TYPE_CHECK (NODE)->type_non_common.values)
 
-#define TYPE_METHODS(NODE) \
-  (RECORD_OR_UNION_CHECK (NODE)->type_non_common.maxval)
 #define TYPE_VFIELD(NODE) \
   (RECORD_OR_UNION_CHECK (NODE)->type_non_common.minval)
 #define TYPE_METHOD_BASETYPE(NODE) \

Re: [PATCH] Kill TYPE_METHODS 0/9

[Jakub Jelinek]

On Fri, Jul 14, 2017 at 12:44:08PM -0400, Nathan Sidwell wrote:
> This is a series of patches that remove the TYPE_METHODS field used in
> records & unions.  Currently TYPE_METHODS hods a the member functions (be
> they static or non-static), and TYPE_FIELDS holds everything else (be they
> FIELD_DECLS or whatever).  This distinction is unnecessary, and the patches
> move everything to TYPE_FIELDS.  (I do not mess with name lookup, which is
> handled differently).
> 
> I do not repurpose TYPE_METHODS, that's later.
> 
> While the changes are pretty mechanical, some are rather too large outside
> of the C++ FE to comfortably apply the obvious rule.
> 
> 1 method-debug.diff - dbxout & dwarf2out.  Review please.
> 2 method-ipa.diff - lto-devirt.  Review please.
> 3 method-rtl.diff - most odd occurrence.  Comment please.
> 
> 4 method-ada.diff - ada-spec generation.  Obvious.
> 5 method-cp.diff - C++ FE changes, self reviewed
> 6 method-libcc1.diff - libcp1plugin.  Obvious.
> 7 method-misc.diff - random tree.c.  Obvious.
> 8 method-objc.diff - objc.  Obvious
> 
> 9 method-ectomy.diff - delete the macro.  Obvious

Seems TYPE_METHODS have been left in a couple of spots.  For winnt-cxx.c
it apparently causes bootstrap failure (I have no way to test it for that
target, but given that the bootstrap is certainly broken right now, it
can't make things worse) and documentation has not been updated.

Ok for trunk?

2017-07-24  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/81521
	* tree.def: Remove TYPE_METHODS documentation, adjust TYPE_FIELDS
	documentation.
	* doc/generic.texi: Likewise.
	* config/i386/winnt-cxx.c (i386_pe_adjust_class_at_definition): Look
	for FUNCTION_DECLs in TYPE_FIELDS rather than TYPE_METHODS.

--- gcc/tree.def.jj	2017-01-01 12:45:36.000000000 +0100
+++ gcc/tree.def	2017-07-24 11:10:11.782877862 +0200
@@ -112,10 +112,7 @@ DEFTREECODE (BLOCK, "block", tcc_excepti
     itself or have named members doesn't really have a "scope" per se.
   The TYPE_STUB_DECL field is used as a forward-references to names for
     ENUMERAL_TYPE, RECORD_TYPE, UNION_TYPE, and QUAL_UNION_TYPE nodes;
-    see below.
-  The TYPE_METHODS points to list of all methods associated with the type.
-    It is non-NULL only at main variant of the type and after free_lang_data
-    it may be set to error_mark_node instead of actual list to save memory. */
+    see below.  */
 
 /* The ordering of the following codes is optimized for the checking
    macros in tree.h.  Changing the order will degrade the speed of the
@@ -213,8 +210,9 @@ DEFTREECODE (ARRAY_TYPE, "array_type", t
 /* Struct in C, or record in Pascal.  */
 /* Special fields:
    TYPE_FIELDS  chain of FIELD_DECLs for the fields of the struct,
-     and VAR_DECLs, TYPE_DECLs and CONST_DECLs for record-scope variables,
-     types and enumerators.
+     VAR_DECLs, TYPE_DECLs and CONST_DECLs for record-scope variables,
+     types and enumerators and FUNCTION_DECLs for methods associated
+     with the type.
    A few may need to be added for Pascal.  */
 /* See the comment above, before ENUMERAL_TYPE, for how
    forward references to struct tags are handled in C.  */
--- gcc/doc/generic.texi.jj	2017-01-01 12:45:47.000000000 +0100
+++ gcc/doc/generic.texi	2017-07-24 11:07:37.331728909 +0200
@@ -2820,7 +2820,6 @@ This function cannot be used with namesp
 @findex BINFO_TYPE
 @findex TYPE_FIELDS
 @findex TYPE_VFIELD
-@findex TYPE_METHODS
 
 Besides namespaces, the other high-level scoping construct in C++ is the
 class.  (Throughout this manual the term @dfn{class} is used to mean the
@@ -2837,7 +2836,7 @@ macro to discern whether or not a partic
 opposed to a @code{struct}.  This macro will be true only for classes
 declared with the @code{class} tag.
 
-Almost all non-function members are available on the @code{TYPE_FIELDS}
+Almost all members are available on the @code{TYPE_FIELDS}
 list.  Given one member, the next can be found by following the
 @code{TREE_CHAIN}.  You should not depend in any way on the order in
 which fields appear on this list.  All nodes on this list will be
@@ -2849,7 +2848,11 @@ list, if the enumeration type was declar
 the @code{TYPE_DECL} for the enumeration type will appear here as well.)
 There are no entries for base classes on this list.  In particular,
 there is no @code{FIELD_DECL} for the ``base-class portion'' of an
-object.
+object.  If a function member is overloaded, each of the overloaded
+functions appears; no @code{OVERLOAD} nodes appear on the @code{TYPE_FIELDS}
+list.  Implicitly declared functions (including default constructors,
+copy constructors, assignment operators, and destructors) will appear on
+this list as well.
 
 The @code{TYPE_VFIELD} is a compiler-generated field used to point to
 virtual function tables.  It may or may not appear on the
@@ -2857,14 +2860,6 @@ virtual function tables.  It may or may
 @code{TYPE_VFIELD} just like all the entries on the @code{TYPE_FIELDS}
 list.
 
-The function members are available on the @code{TYPE_METHODS} list.
-Again, subsequent members are found by following the @code{TREE_CHAIN}
-field.  If a function is overloaded, each of the overloaded functions
-appears; no @code{OVERLOAD} nodes appear on the @code{TYPE_METHODS}
-list.  Implicitly declared functions (including default constructors,
-copy constructors, assignment operators, and destructors) will appear on
-this list as well.
-
 Every class has an associated @dfn{binfo}, which can be obtained with
 @code{TYPE_BINFO}.  Binfos are used to represent base-classes.  The
 binfo given by @code{TYPE_BINFO} is the degenerate case, whereby every
--- gcc/config/i386/winnt-cxx.c.jj	2017-01-01 12:45:42.000000000 +0100
+++ gcc/config/i386/winnt-cxx.c	2017-07-24 11:12:23.723296309 +0200
@@ -114,14 +114,11 @@ i386_pe_adjust_class_at_definition (tree
 	  decl_attributes (&ti_decl, na, 0);
 	}
 
-      /* Check static VAR_DECL's.  */
+      /* Check FUNCTION_DECL's and static VAR_DECL's.  */
       for (member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member))
 	if (TREE_CODE (member) == VAR_DECL)     
 	  maybe_add_dllexport (member);
-    
-      /* Check FUNCTION_DECL's.  */
-      for (member = TYPE_METHODS (t); member;  member = DECL_CHAIN (member))
-	if (TREE_CODE (member) == FUNCTION_DECL)
+	else if (TREE_CODE (member) == FUNCTION_DECL)
 	  {
 	    tree thunk;
 	    maybe_add_dllexport (member);
@@ -132,7 +129,8 @@ i386_pe_adjust_class_at_definition (tree
 	      maybe_add_dllexport (thunk);
 	}
       /* Check vtables  */
-      for (member = CLASSTYPE_VTABLES (t); member;  member = DECL_CHAIN (member))
+      for (member = CLASSTYPE_VTABLES (t);
+	   member; member = DECL_CHAIN (member))
 	if (TREE_CODE (member) == VAR_DECL) 
 	  maybe_add_dllexport (member);
     }

	Jakub

Re: [PATCH] Kill TYPE_METHODS 0/9

[Nathan Sidwell]

On 07/24/2017 05:25 AM, Jakub Jelinek wrote:
> On Fri, Jul 14, 2017 at 12:44:08PM -0400, Nathan Sidwell wrote:

> Seems TYPE_METHODS have been left in a couple of spots.  For winnt-cxx.c
> it apparently causes bootstrap failure (I have no way to test it for that
> target, but given that the bootstrap is certainly broken right now, it
> can't make things worse) and documentation has not been updated.

Thanks for addressing what I overlooked.  Your diff looks good to me.

nathan

-- 
Nathan Sidwell

Re: [PATCH] Kill TYPE_METHODS 0/9

[Jim Wilson]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On 07/24/2017 02:25 AM, Jakub Jelinek wrote:
> Seems TYPE_METHODS have been left in a couple of spots.  For winnt-cxx.c
> it apparently causes bootstrap failure (I have no way to test it for that
> target, but given that the bootstrap is certainly broken right now, it
> can't make things worse) and documentation has not been updated.

There were two uses of TYPE_METHODS in winnt-cxx.c, and Jakub's patch 
fixed only one of them.  I wrote a patch to fix the other, fixed the 
exact same way as Jakub's earlier patch, with a few indentation and 
white space fixes also.  This was tested with a cross build, and checked 
in under the obvious rule.

Jim


[-- Attachment #2: winnt-cxx.patch --]
[-- Type: text/x-patch, Size: 1865 bytes --]

2017-07-25  Jim Wilson  <jim.wilson@linaro.org>

	gcc/
	PR bootstrap/81521
	* config/i386/winnt-cxx.c (i386_pe_adjust_class_at_definition): Look
	for FUNCTION_DECLs in TYPE_FIELDS rather than TYPE_METHODS.

Index: gcc/config/i386/winnt-cxx.c
===================================================================
--- gcc/config/i386/winnt-cxx.c	(revision 250526)
+++ gcc/config/i386/winnt-cxx.c	(working copy)
@@ -127,7 +127,8 @@ i386_pe_adjust_class_at_definition (tree t)
 	    for (thunk = DECL_THUNKS (member); thunk;
 		 thunk = TREE_CHAIN (thunk))
 	      maybe_add_dllexport (thunk);
-	}
+	  }
+
       /* Check vtables  */
       for (member = CLASSTYPE_VTABLES (t);
 	   member; member = DECL_CHAIN (member))
@@ -145,14 +146,11 @@ i386_pe_adjust_class_at_definition (tree t)
 	 That is just right since out-of class declarations can only be a
 	 definition.   */
 
-      /* Check static VAR_DECL's.  */
+      /* Check FUNCTION_DECL's and static VAR_DECL's.  */
       for (member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member))
 	if (TREE_CODE (member) == VAR_DECL)     
 	  maybe_add_dllimport (member);
-    
-      /* Check FUNCTION_DECL's.  */
-      for (member = TYPE_METHODS (t); member;  member = DECL_CHAIN (member))
-	if (TREE_CODE (member) == FUNCTION_DECL)
+	else if (TREE_CODE (member) == FUNCTION_DECL)
 	  {
 	    tree thunk;
 	    maybe_add_dllimport (member);
@@ -161,10 +159,11 @@ i386_pe_adjust_class_at_definition (tree t)
 	    for (thunk = DECL_THUNKS (member); thunk;
 		 thunk = DECL_CHAIN (thunk))
 	      maybe_add_dllimport (thunk);
-	 }
+	  }
  
       /* Check vtables  */
-      for (member = CLASSTYPE_VTABLES (t); member;  member = DECL_CHAIN (member))
+      for (member = CLASSTYPE_VTABLES (t);
+	   member;  member = DECL_CHAIN (member))
 	if (TREE_CODE (member) == VAR_DECL) 
 	  maybe_add_dllimport (member);
 

Re: [PATCH] Kill TYPE_METHODS 0/9

[Jim Wilson]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On 07/24/2017 02:25 AM, Jakub Jelinek wrote:
> Seems TYPE_METHODS have been left in a couple of spots.  For winnt-cxx.c
> it apparently causes bootstrap failure (I have no way to test it for that
> target, but given that the bootstrap is certainly broken right now, it
> can't make things worse) and documentation has not been updated.

There were two uses of TYPE_METHODS in winnt-cxx.c, and Jakub's patch 
fixed only one of them.  I wrote a patch to fix the other, fixed the 
exact same way as Jakub's earlier patch, with a few indentation and 
white space fixes also.  This was tested with a cross build, and checked 
in under the obvious rule.

Jim


[-- Attachment #2: winnt-cxx.patch --]
[-- Type: text/x-patch, Size: 1865 bytes --]

2017-07-25  Jim Wilson  <jim.wilson@linaro.org>

	gcc/
	PR bootstrap/81521
	* config/i386/winnt-cxx.c (i386_pe_adjust_class_at_definition): Look
	for FUNCTION_DECLs in TYPE_FIELDS rather than TYPE_METHODS.

Index: gcc/config/i386/winnt-cxx.c
===================================================================
--- gcc/config/i386/winnt-cxx.c	(revision 250526)
+++ gcc/config/i386/winnt-cxx.c	(working copy)
@@ -127,7 +127,8 @@ i386_pe_adjust_class_at_definition (tree t)
 	    for (thunk = DECL_THUNKS (member); thunk;
 		 thunk = TREE_CHAIN (thunk))
 	      maybe_add_dllexport (thunk);
-	}
+	  }
+
       /* Check vtables  */
       for (member = CLASSTYPE_VTABLES (t);
 	   member; member = DECL_CHAIN (member))
@@ -145,14 +146,11 @@ i386_pe_adjust_class_at_definition (tree t)
 	 That is just right since out-of class declarations can only be a
 	 definition.   */
 
-      /* Check static VAR_DECL's.  */
+      /* Check FUNCTION_DECL's and static VAR_DECL's.  */
       for (member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member))
 	if (TREE_CODE (member) == VAR_DECL)     
 	  maybe_add_dllimport (member);
-    
-      /* Check FUNCTION_DECL's.  */
-      for (member = TYPE_METHODS (t); member;  member = DECL_CHAIN (member))
-	if (TREE_CODE (member) == FUNCTION_DECL)
+	else if (TREE_CODE (member) == FUNCTION_DECL)
 	  {
 	    tree thunk;
 	    maybe_add_dllimport (member);
@@ -161,10 +159,11 @@ i386_pe_adjust_class_at_definition (tree t)
 	    for (thunk = DECL_THUNKS (member); thunk;
 		 thunk = DECL_CHAIN (thunk))
 	      maybe_add_dllimport (thunk);
-	 }
+	  }
  
       /* Check vtables  */
-      for (member = CLASSTYPE_VTABLES (t); member;  member = DECL_CHAIN (member))
+      for (member = CLASSTYPE_VTABLES (t);
+	   member;  member = DECL_CHAIN (member))
 	if (TREE_CODE (member) == VAR_DECL) 
 	  maybe_add_dllimport (member);