public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* gengtype & plugins [#define emitted in wrong order]
@ 2009-09-30 21:39 Basile STARYNKEVITCH
  2009-10-01  6:06 ` Basile STARYNKEVITCH
  0 siblings, 1 reply; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-09-30 21:39 UTC (permalink / raw)
  To: gcc-patches

Hello All,

A big thanks for the gengtype patches for plugin finalized by
Rafael Avila de Espindola  <espindola@google.com>

See also
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02104.html
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02057.html

The current gengtype of trunk 152325 is now able to run correctly for 
MELT. It does not crash, thanks to Rafael!


However, it seems to me that in plugin mode, the #define are emitted in 
the wrong order. To be specific, in plugin mode, the generated 
gt-melt-runtime.h with

  /usr/src/Lang/_Boot_Gcctrunk/gcc/build/gengtype -P gt-melt-runtime.h \
	      /usr/src/Lang/gcc-trunk-bstarynk/gcc our-gtyp-input.list \
	      melt-runtime.h melt-runtime.c

where /usr/src/Lang/_Boot_Gcctrunk is the GCC trunk build directory, and
/usr/src/Lang/gcc-trunk-bstarynk/ is the GCC trunk source directory

contains first at line 28 to 46

void
gt_ggc_mx_VEC_melt_ptr_t_gc (void *x_p)
{
   struct VEC_melt_ptr_t_gc * const x = (struct VEC_melt_ptr_t_gc *)x_p;
   if (ggc_test_and_set_mark (x))
     {
       {
         size_t i0;
         size_t l0 = (size_t)(((*x).base).num);
         for (i0 = 0; i0 != l0; i0++) {
           gt_ggc_m_7melt_un ((*x).base.vec[i0]);
         }
       }
     }
}

and only next at line 106 to 109
#define gt_ggc_m_7melt_un(X) do { \
   if (X != NULL) gt_ggc_mx_melt_un (X);\
   } while (0)
extern void gt_ggc_mx_melt_un (void *);


This is clearly incorrect. To make it short, in plugin mode, all the 
generated #define gt_ggc_*
should be output before the routine code.

If you want to reproduce the bug, follow instructions in
http://gcc.gnu.org/wiki/MELT%20tutorial section building and using MELT 
as a plugin of course taking the trunk's gengtype!


I will investigate later (perhaps in 2 days) the details. I suspect a 
simple change in gengtype.c should suffice, so I hope such a change is 
acceptable in stage 3, since it is really a bug fix.


Regards.

PS. I hope this does belong to gcc-patches@ since we are discussing 
recent patches & commits. I am sorry to not provide a correction here.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-09-30 21:39 gengtype & plugins [#define emitted in wrong order] Basile STARYNKEVITCH
@ 2009-10-01  6:06 ` Basile STARYNKEVITCH
  2009-10-01  6:34   ` Basile STARYNKEVITCH
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-01  6:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: Rafael Espindola, Diego Novillo

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

Hello All

I (=Basile STARYNKEVITCH) wrote in 
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02274.html

> Hello All,
> 
> A big thanks for the gengtype patches for plugin finalized by
> Rafael Avila de Espindola  <espindola@google.com>
> 
> See also
> http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02104.html
> http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02057.html
> 
> The current gengtype of trunk 152325 is now able to run correctly for 
> MELT. It does not crash, thanks to Rafael!
> 
> 
> However, it seems to me that in plugin mode, the #define are emitted in 
> the wrong order. 

Correcting the problem is actually simple, as the attached patch 
(against trunk r152356) demonstrates. I just moved the 
write_func_for_structure calls inside write_types into their own loops.

I am also attaching the gcc/ChangeLog entry

Ok for trunk if it bootstraps?

[I won't have internet access for the next 10 hours, so Rafael or Diego 
or anyone else could commit the patch if approved]

Regards

PS: I really hope this small kind of bugfix fits in stage 3. Should I 
make a bug report decribing 
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02274.html in bugzilla?

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 244 bytes --]

2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype.c (write_types): Moved call to write_func_for_structure
	into seperate loops to ensure that all the functions are emitted
	after their declarations and #define-d macros.

[-- Attachment #3: patch-gengtype-plugin-r152356.diff --]
[-- Type: text/x-patch, Size: 2279 bytes --]

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 152357)
+++ gcc/gengtype.c	(working copy)
@@ -2719,6 +2719,9 @@ write_types (outf_p output_header, type_p structur
   type_p s;
 
   oprintf (output_header, "\n/* %s*/\n", wtd->comment);
+  /* We first emit the macros and the declarations. Function codes is
+     emitted afterward. */
+  oprintf (output_header, "/* macros and declarations */\n");
   for (s = structures; s; s = s->next)
     if (s->gc_used == GC_POINTED_TO
 	|| s->gc_used == GC_MAYBE_POINTED_TO)
@@ -2767,21 +2770,11 @@ write_types (outf_p output_header, type_p structur
 		     s->u.s.tag);
 	    continue;
 	  }
-
-	if (s->kind == TYPE_LANG_STRUCT)
-	  {
-	    type_p ss;
-	    for (ss = s->u.s.lang_struct; ss; ss = ss->next)
-	      write_func_for_structure (s, ss, NULL, wtd);
 	  }
-	else
-	  write_func_for_structure (s, s, NULL, wtd);
-      }
 
   for (s = param_structs; s; s = s->next)
     if (s->gc_used == GC_POINTED_TO)
       {
-	type_p * param = s->u.param_struct.param;
 	type_p stru = s->u.param_struct.stru;
 
 	/* Declare the marker procedure.  */
@@ -2795,7 +2788,41 @@ write_types (outf_p output_header, type_p structur
 		     s->u.s.tag);
 	    continue;
 	  }
+      }
 
+  /* At last we emit the functions code. */ 
+  oprintf (output_header, "\n/* functions code */\n");
+  for (s = structures; s; s = s->next)
+    if (s->gc_used == GC_POINTED_TO
+	|| s->gc_used == GC_MAYBE_POINTED_TO)
+      {
+	options_p opt;
+
+	if (s->gc_used == GC_MAYBE_POINTED_TO
+	    && s->u.s.line.file == NULL)
+	  continue;
+	for (opt = s->u.s.opt; opt; opt = opt->next)
+	  if (strcmp (opt->name, "ptr_alias") == 0)
+	    break;
+	if (opt)
+	  continue;
+	
+	if (s->kind == TYPE_LANG_STRUCT)
+	  {
+	    type_p ss;
+	    for (ss = s->u.s.lang_struct; ss; ss = ss->next)
+	      write_func_for_structure (s, ss, NULL, wtd);
+	  }
+	else
+	  write_func_for_structure (s, s, NULL, wtd);
+      };
+  for (s = param_structs; s; s = s->next)
+    if (s->gc_used == GC_POINTED_TO)
+      {
+	type_p * param = s->u.param_struct.param;
+	type_p stru = s->u.param_struct.stru;
+	if (stru->u.s.line.file == NULL)
+	    continue;
 	if (stru->kind == TYPE_LANG_STRUCT)
 	  {
 	    type_p ss;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-01  6:06 ` Basile STARYNKEVITCH
@ 2009-10-01  6:34   ` Basile STARYNKEVITCH
  2009-10-01 19:52   ` Rafael Espindola
  2009-10-02 21:02   ` Gerald Pfeifer
  2 siblings, 0 replies; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-01  6:34 UTC (permalink / raw)
  To: gcc-patches; +Cc: Rafael Espindola, Diego Novillo

Basile STARYNKEVITCH wrote:
>> However, it seems to me that in plugin mode, the #define are emitted 
>> in the wrong order. 
> 
> Correcting the problem is actually simple, as the attached patch 
> (against trunk r152356) demonstrates. I just moved the 
> write_func_for_structure calls inside write_types into their own loops.
> 
> I am also attaching the gcc/ChangeLog entry
> 
> Ok for trunk if it bootstraps?

It did bootstrap on amd64 debian sid (linux x86-64).

Regards.
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-01  6:06 ` Basile STARYNKEVITCH
  2009-10-01  6:34   ` Basile STARYNKEVITCH
@ 2009-10-01 19:52   ` Rafael Espindola
  2009-10-01 20:33     ` Basile STARYNKEVITCH
  2009-10-02 21:02   ` Gerald Pfeifer
  2 siblings, 1 reply; 16+ messages in thread
From: Rafael Espindola @ 2009-10-01 19:52 UTC (permalink / raw)
  To: Basile STARYNKEVITCH; +Cc: gcc-patches, Diego Novillo

> Correcting the problem is actually simple, as the attached patch (against
> trunk r152356) demonstrates. I just moved the write_func_for_structure calls
> inside write_types into their own loops.
>
> I am also attaching the gcc/ChangeLog entry
>
> Ok for trunk if it bootstraps?

The patch looks reasonable, what I don't understand is why this is a
problem only for plugins. Do you know what makes the old order work
for the non-plugin case?

Some comments:

* The } on line 2773 in unaligned.
* There is a trailing space on the "emit the functions" comment on line 2793.
* There is a dummy ; on line 2818

>
> Regards
>
> PS: I really hope this small kind of bugfix fits in stage 3. Should I make a
> bug report decribing http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02274.html
> in bugzilla?
>


Cheers,
-- 
Rafael Ávila de Espíndola

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-01 19:52   ` Rafael Espindola
@ 2009-10-01 20:33     ` Basile STARYNKEVITCH
  2009-10-01 21:02       ` Rafael Espindola
  0 siblings, 1 reply; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-01 20:33 UTC (permalink / raw)
  To: Rafael Espindola; +Cc: gcc-patches, Diego Novillo

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

Rafael Espindola wrote, citing first me (Basile)
>> Correcting the problem is actually simple, as the attached patch (against
>> trunk r152356) demonstrates. I just moved the write_func_for_structure calls
>> inside write_types into their own loops.
>>
>> I am also attaching the gcc/ChangeLog entry
>>
>> Ok for trunk if it bootstraps?
> 
> The patch looks reasonable, what I don't understand is why this is a
> problem only for plugins. Do you know what makes the old order work
> for the non-plugin case?

I don't claim to have a complete & detailed theory of gengtype's 
behavior (in other words I don't think I understand every detail of it; 
actually I would dare to believe that nobody understand entirely 
gengtype!). However, with the MELT branch, I observe that every macro is 
defined inside the common generated file gtype-desc.h which is included 
by a lot of files, before any code. And the generated gtype-desc.c 
obviously #include gtype-desc.h (and all the include files listed in 
array ifiles in function open_base_file of gengtype.c) before any code. 
A similar observation holds for the trunk itself, so I would believe 
that the macros are probably always generated in gtype-desc.h, and that 
does explain that we did not met that "bug" before.

> 
> Some comments:
> 
> * The } on line 2773 in unaligned.
> * There is a trailing space on the "emit the functions" comment on line 2793.
> * There is a dummy ; on line 2818
> 

Thanks. I am attaching a slightly enhanced patch to trunk rev 152380 
(and the gcc/ChangeLog which has not changed); the only differences are 
the typos above and a few words in comments.

Ok if it bootstraps? (on Debian Sid Amd64 ie gnu linux x86-64)


Regards.

BTW, I produce the patch diff file with
svn diff -x -p -x -b gcc/gengtype.c > \ 
~/tmp/patch-gengtype-plugin-r152380.diff
Is that command the right way to produce such a patch?
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

[-- Attachment #2: patch-gengtype-plugin-r152380.diff --]
[-- Type: text/x-patch, Size: 3470 bytes --]

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 152380)
+++ gcc/gengtype.c	(working copy)
@@ -128,7 +128,7 @@
 
 /* An output file, suitable for definitions, that can see declarations
    made in INPUT_FILE and is linked into every language that uses
-   INPUT_FILE.  May return NULL in plugin mode. */
+   INPUT_FILE.  May return NULL in plug-in mode. */
 extern outf_p get_output_file_with_visibility
    (const char *input_file);
 const char *get_output_file_name (const char *);
@@ -140,12 +140,11 @@
 /* The list of output files.  */
 static outf_p output_files;
 
-/* The plugin input files and their number; in that case only
-   corresponding gt-<plugin>.h are generated in the current
-   directory.  */
+/* The plug-in input files and their number; in that case only
+   a single file is produced.  */
 static char** plugin_files;
 static size_t nb_plugin_files;
-/* the generated plugin output name & file */
+/* the generated plug-in output name & file */
 static outf_p plugin_output;
 
 /* The output header file that is included into pretty much every
@@ -463,7 +462,7 @@
       /* Update the global counts now that we know accurately how many
 	 things there are.  (We do not bother resizing the arrays down.)  */
       num_lang_dirs = langno;
-      /* Add the plugin files if provided.  */
+      /* Add the plug-in files if provided.  */
       if (plugin_files) 
 	{
 	  size_t i;
@@ -2719,6 +2718,9 @@
   type_p s;
 
   oprintf (output_header, "\n/* %s*/\n", wtd->comment);
+  /* We first emit the macros and the declarations. Functions' code is
+     emitted afterwards.  This is needed in plugin mode. */
+  oprintf (output_header, "/* macros and declarations */\n");
   for (s = structures; s; s = s->next)
     if (s->gc_used == GC_POINTED_TO
 	|| s->gc_used == GC_MAYBE_POINTED_TO)
@@ -2767,21 +2769,11 @@
 		     s->u.s.tag);
 	    continue;
 	  }
-
-	if (s->kind == TYPE_LANG_STRUCT)
-	  {
-	    type_p ss;
-	    for (ss = s->u.s.lang_struct; ss; ss = ss->next)
-	      write_func_for_structure (s, ss, NULL, wtd);
 	  }
-	else
-	  write_func_for_structure (s, s, NULL, wtd);
-      }
 
   for (s = param_structs; s; s = s->next)
     if (s->gc_used == GC_POINTED_TO)
       {
-	type_p * param = s->u.param_struct.param;
 	type_p stru = s->u.param_struct.stru;
 
 	/* Declare the marker procedure.  */
@@ -2795,7 +2787,41 @@
 		     s->u.s.tag);
 	    continue;
 	  }
+      }
+  
+  /* At last we emit the functions code.*/ 
+  oprintf (output_header, "\n/* functions code */\n");
+  for (s = structures; s; s = s->next)
+    if (s->gc_used == GC_POINTED_TO
+	|| s->gc_used == GC_MAYBE_POINTED_TO)
+      {
+	options_p opt;
 
+	if (s->gc_used == GC_MAYBE_POINTED_TO
+	    && s->u.s.line.file == NULL)
+	  continue;
+	for (opt = s->u.s.opt; opt; opt = opt->next)
+	  if (strcmp (opt->name, "ptr_alias") == 0)
+	    break;
+	if (opt)
+	  continue;
+	
+	if (s->kind == TYPE_LANG_STRUCT)
+	  {
+	    type_p ss;
+	    for (ss = s->u.s.lang_struct; ss; ss = ss->next)
+	      write_func_for_structure (s, ss, NULL, wtd);
+	  }
+	else
+	  write_func_for_structure (s, s, NULL, wtd);
+      }
+  for (s = param_structs; s; s = s->next)
+    if (s->gc_used == GC_POINTED_TO)
+      {
+	type_p * param = s->u.param_struct.param;
+	type_p stru = s->u.param_struct.stru;
+	if (stru->u.s.line.file == NULL)
+	  continue;
 	if (stru->kind == TYPE_LANG_STRUCT)
 	  {
 	    type_p ss;

[-- Attachment #3: ChangeLog --]
[-- Type: text/plain, Size: 244 bytes --]

2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype.c (write_types): Moved call to write_func_for_structure
	into seperate loops to ensure that all the functions are emitted
	after their declarations and #define-d macros.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-01 20:33     ` Basile STARYNKEVITCH
@ 2009-10-01 21:02       ` Rafael Espindola
  0 siblings, 0 replies; 16+ messages in thread
From: Rafael Espindola @ 2009-10-01 21:02 UTC (permalink / raw)
  To: Basile STARYNKEVITCH; +Cc: gcc-patches, Diego Novillo

> I don't claim to have a complete & detailed theory of gengtype's behavior
> (in other words I don't think I understand every detail of it; actually I
> would dare to believe that nobody understand entirely gengtype!). However,
> with the MELT branch, I observe that every macro is defined inside the
> common generated file gtype-desc.h which is included by a lot of files,
> before any code. And the generated gtype-desc.c obviously #include
> gtype-desc.h (and all the include files listed in array ifiles in function
> open_base_file of gengtype.c) before any code. A similar observation holds
> for the trunk itself, so I would believe that the macros are probably always
> generated in gtype-desc.h, and that does explain that we did not met that
> "bug" before.

Makes sense. Thanks.

> Thanks. I am attaching a slightly enhanced patch to trunk rev 152380 (and
> the gcc/ChangeLog which has not changed); the only differences are the typos
> above and a few words in comments.
>
> Ok if it bootstraps? (on Debian Sid Amd64 ie gnu linux x86-64)

I think it is OK, but not 100% sure if my "plugin review" powers cover this :-)
Diego, can you take a look?

>
> Regards.
>
> BTW, I produce the patch diff file with
> svn diff -x -p -x -b gcc/gengtype.c > \
> ~/tmp/patch-gengtype-plugin-r152380.diff
> Is that command the right way to produce such a patch?

It produces a correct looking patch :-)
You might find it convenient to setup svn to use an external diff program:
http://gcc.gnu.org/wiki/SvnSetup

> --
> Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
> email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
> 8, rue de la Faiencerie, 92340 Bourg La Reine, France
> *** opinions {are only mines, sont seulement les miennes} ***
>
> 2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>
>
>        * gengtype.c (write_types): Moved call to write_func_for_structure
>        into seperate loops to ensure that all the functions are emitted
>        after their declarations and #define-d macros.
>
>



Cheers,
-- 
Rafael Ávila de Espíndola

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-01  6:06 ` Basile STARYNKEVITCH
  2009-10-01  6:34   ` Basile STARYNKEVITCH
  2009-10-01 19:52   ` Rafael Espindola
@ 2009-10-02 21:02   ` Gerald Pfeifer
  2009-10-03  5:54     ` Basile STARYNKEVITCH
  2 siblings, 1 reply; 16+ messages in thread
From: Gerald Pfeifer @ 2009-10-02 21:02 UTC (permalink / raw)
  To: Basile STARYNKEVITCH; +Cc: gcc-patches, Rafael Espindola, Diego Novillo

On Thu, 1 Oct 2009, Basile STARYNKEVITCH wrote:
> I am also attaching the gcc/ChangeLog entry

2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype.c (write_types): Moved call to write_func_for_structure
	into seperate loops to ensure that all the functions are emitted
	after their declarations and #define-d macros.

The ChangeLog should only describe what changes, not whay.  The latter
you may want to put as comments in the code.

Gerald

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-02 21:02   ` Gerald Pfeifer
@ 2009-10-03  5:54     ` Basile STARYNKEVITCH
  2009-10-05 15:11       ` Diego Novillo
  0 siblings, 1 reply; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-03  5:54 UTC (permalink / raw)
  To: Gerald Pfeifer; +Cc: gcc-patches, Rafael Espindola, Diego Novillo

Gerald Pfeifer wrote:
> On Thu, 1 Oct 2009, Basile STARYNKEVITCH wrote:
>> I am also attaching the gcc/ChangeLog entry
> 
> 2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>
> 
>     * gengtype.c (write_types): Moved call to write_func_for_structure
>     into seperate loops to ensure that all the functions are emitted
>     after their declarations and #define-d macros.
> 
> The ChangeLog should only describe what changes, not whay.  The latter
> you may want to put as comments in the code.

Thanks for the comment. Hence, the gcc/ChangeLog should be simply:

2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>

      * gengtype.c (write_types): Moved call to write_func_for_structure
      into seperate loops.


With this change in the changelog, is the patch OK for trunk??

Regards.


-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-03  5:54     ` Basile STARYNKEVITCH
@ 2009-10-05 15:11       ` Diego Novillo
  2009-10-05 16:12         ` Basile STARYNKEVITCH
  0 siblings, 1 reply; 16+ messages in thread
From: Diego Novillo @ 2009-10-05 15:11 UTC (permalink / raw)
  To: Basile STARYNKEVITCH; +Cc: Gerald Pfeifer, gcc-patches, Rafael Espindola

On Sat, Oct 3, 2009 at 01:55, Basile STARYNKEVITCH
<basile@starynkevitch.net> wrote:

> 2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>
>
>     * gengtype.c (write_types): Moved call to write_func_for_structure
>     into seperate loops.
>
>
> With this change in the changelog, is the patch OK for trunk??
>
> Index: gcc/gengtype.c
> ===================================================================
> --- gcc/gengtype.c	(revision 152380)
> +++ gcc/gengtype.c	(working copy)
> @@ -128,7 +128,7 @@
>
>  /* An output file, suitable for definitions, that can see declarations
>     made in INPUT_FILE and is linked into every language that uses
> -   INPUT_FILE.  May return NULL in plugin mode. */
> +   INPUT_FILE.  May return NULL in plug-in mode. */

s/plug-in/plugin/ everywhere.

>    oprintf (output_header, "\n/* %s*/\n", wtd->comment);
> +  /* We first emit the macros and the declarations. Functions' code is
> +     emitted afterwards.  This is needed in plugin mode. */

Two spaces before '*/'.

> +      }
> +
> +  /* At last we emit the functions code.*/

Likewise.

> +  for (s = param_structs; s; s = s->next)
> +    if (s->gc_used == GC_POINTED_TO)
> +      {
> +	type_p * param = s->u.param_struct.param;

No space before 'param'.

OK with those changes and testing that the generated files work.


Diego.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-05 15:11       ` Diego Novillo
@ 2009-10-05 16:12         ` Basile STARYNKEVITCH
  2009-10-05 18:33           ` Rafael Espindola
  2009-10-05 19:52           ` Joseph S. Myers
  0 siblings, 2 replies; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-05 16:12 UTC (permalink / raw)
  To: Diego Novillo; +Cc: Gerald Pfeifer, gcc-patches, Rafael Espindola

Diego Novillo wrote:
> On Sat, Oct 3, 2009 at 01:55, Basile STARYNKEVITCH
> <basile@starynkevitch.net> wrote:
> 
>> 2009-10-01  Basile Starynkevitch  <basile@starynkevitch.net>
>>
>>     * gengtype.c (write_types): Moved call to write_func_for_structure
>>     into seperate loops.
>>
>>
>> With this change in the changelog, is the patch OK for trunk??

> OK with those changes and testing that the generated files work.


gengtype works ok for the trunk, and the patch works for MELT as a 
plugin (using the same gengtype in plugin mode).

A warm thanks to Diego for the review and the patience, and to Rafael 
Espindola for the work.

I also added Rafael Espindola's name&email  in the gcc/ChangeLog entry.


2009-10-05  Basile Starynkevitch  <basile@starynkevitch.net>
	    Rafael Espindola  <espindola@google.com>

	* gengtype.c (write_types): Moved call to write_func_for_structure
	into seperate loops.

I hope Rafael is not offended by being named here... (He did most of 
gengtype improvement after my initial patch).

I am very happy to have committed that as revision 152455, because for 
the very first time, MELT can be compiled as an ordinary GCC plugin 
(previously, you had to use MELT's gengtype - which had this correction).

BTW, I tend to believe that gengtype might be installed somewhere 
(perhaps as gcc-gengtype), and we could also want to put the version 
string of GCC inside gengtype binary (or perhaps is it already there?). 
We might also want to have gengtype accepting more a GNU way for program 
arguments (probably at least  have gengtype accepting --version & --help).

Regards.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-05 16:12         ` Basile STARYNKEVITCH
@ 2009-10-05 18:33           ` Rafael Espindola
  2009-10-05 19:52           ` Joseph S. Myers
  1 sibling, 0 replies; 16+ messages in thread
From: Rafael Espindola @ 2009-10-05 18:33 UTC (permalink / raw)
  To: Basile STARYNKEVITCH; +Cc: Diego Novillo, Gerald Pfeifer, gcc-patches

> BTW, I tend to believe that gengtype might be installed somewhere (perhaps
> as gcc-gengtype), and we could also want to put the version string of GCC
> inside gengtype binary (or perhaps is it already there?). We might also want
> to have gengtype accepting more a GNU way for program arguments (probably at
> least  have gengtype accepting --version & --help).

Those all look like nice improvements, and might be small enough to
fit in stage3. I just don't think they are very high priority.

> Regards.
>

Cheers,
-- 
Rafael Ávila de Espíndola

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-05 16:12         ` Basile STARYNKEVITCH
  2009-10-05 18:33           ` Rafael Espindola
@ 2009-10-05 19:52           ` Joseph S. Myers
  2009-10-06  8:32             ` Basile STARYNKEVITCH
  1 sibling, 1 reply; 16+ messages in thread
From: Joseph S. Myers @ 2009-10-05 19:52 UTC (permalink / raw)
  To: Basile STARYNKEVITCH
  Cc: Diego Novillo, Gerald Pfeifer, gcc-patches, Rafael Espindola

On Mon, 5 Oct 2009, Basile STARYNKEVITCH wrote:

> BTW, I tend to believe that gengtype might be installed somewhere (perhaps as
> gcc-gengtype), and we could also want to put the version string of GCC inside
> gengtype binary (or perhaps is it already there?). We might also want to have
> gengtype accepting more a GNU way for program arguments (probably at least
> have gengtype accepting --version & --help).

gengtype is presently built only for the build system.  If you want to 
install it you'll need to build it for the host and install that (and, 
yes, support --help and --version with the usual adaptation to the 
--with-bugurl and --with-pkgversion options).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-05 19:52           ` Joseph S. Myers
@ 2009-10-06  8:32             ` Basile STARYNKEVITCH
  2009-10-06 13:24               ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-06  8:32 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Diego Novillo, Gerald Pfeifer, gcc-patches, Rafael Espindola

Joseph S. Myers wrote:
> On Mon, 5 Oct 2009, Basile STARYNKEVITCH wrote:
> 
>> BTW, I tend to believe that gengtype might be installed somewhere (perhaps as
>> gcc-gengtype), and we could also want to put the version string of GCC inside
>> gengtype binary (or perhaps is it already there?). We might also want to have
>> gengtype accepting more a GNU way for program arguments (probably at least
>> have gengtype accepting --version & --help).
> 
> gengtype is presently built only for the build system.  If you want to 
> install it you'll need to build it for the host and install that (and, 
> yes, support --help and --version with the usual adaptation to the 
> --with-bugurl and --with-pkgversion options).


I have absolutely no idea how to implement all that -and today it is not 
my topmost priority! What would be the code model to follow?

More generally, future linux distributions should probably have a 
gcc-dev package for developping GCC plugins & extensions, much like they 
have a firefox-dev or a gedit-dev for developping plugins for firefox or 
gedit. Perhaps the GCC community should help on such gcc-dev package (or 
  maybe not?). We might want to facilitate GCC plugins development.

Regards.


-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-06  8:32             ` Basile STARYNKEVITCH
@ 2009-10-06 13:24               ` Joseph S. Myers
  2009-10-06 13:52                 ` Basile STARYNKEVITCH
  0 siblings, 1 reply; 16+ messages in thread
From: Joseph S. Myers @ 2009-10-06 13:24 UTC (permalink / raw)
  To: Basile STARYNKEVITCH
  Cc: Diego Novillo, Gerald Pfeifer, gcc-patches, Rafael Espindola

On Tue, 6 Oct 2009, Basile STARYNKEVITCH wrote:

> > gengtype is presently built only for the build system.  If you want to
> > install it you'll need to build it for the host and install that (and, yes,
> > support --help and --version with the usual adaptation to the --with-bugurl
> > and --with-pkgversion options).
> 
> I have absolutely no idea how to implement all that -and today it is not my
> topmost priority! What would be the code model to follow?

There are plenty of examples in GCC of programs handling --help and 
--version; if you want something simpler than the gcc driver, you could 
look at gcov, for example.  Various files used in generator programs and 
in GCC proper are already built for both host and build systems.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-06 13:24               ` Joseph S. Myers
@ 2009-10-06 13:52                 ` Basile STARYNKEVITCH
  2009-10-06 14:36                   ` Joseph S. Myers
  0 siblings, 1 reply; 16+ messages in thread
From: Basile STARYNKEVITCH @ 2009-10-06 13:52 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Diego Novillo, Gerald Pfeifer, gcc-patches, Rafael Espindola

Joseph S. Myers wrote:
> On Tue, 6 Oct 2009, Basile STARYNKEVITCH wrote:
> 
>>> gengtype is presently built only for the build system.  If you want to
>>> install it you'll need to build it for the host and install that (and, yes,
>>> support --help and --version with the usual adaptation to the --with-bugurl
>>> and --with-pkgversion options).
>> I have absolutely no idea how to implement all that -and today it is not my
>> topmost priority! What would be the code model to follow?
> 
> There are plenty of examples in GCC of programs handling --help and 
> --version; if you want something simpler than the gcc driver, you could 
> look at gcov, for example.  Various files used in generator programs and 
> in GCC proper are already built for both host and build systems.


Sorry, I badly explained why I don't understand.

What I don't understand is how can gengtype retrieve the exact version 
string of GCC, and the pkgversion. I also am not sure to grasp what 
exactly -with-pkgversion is for. Does it have something in common with 
pkg-config utility?

Regards.


-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: gengtype & plugins [#define emitted in wrong order]
  2009-10-06 13:52                 ` Basile STARYNKEVITCH
@ 2009-10-06 14:36                   ` Joseph S. Myers
  0 siblings, 0 replies; 16+ messages in thread
From: Joseph S. Myers @ 2009-10-06 14:36 UTC (permalink / raw)
  To: Basile STARYNKEVITCH
  Cc: Diego Novillo, Gerald Pfeifer, gcc-patches, Rafael Espindola

On Tue, 6 Oct 2009, Basile STARYNKEVITCH wrote:

> Joseph S. Myers wrote:
> > On Tue, 6 Oct 2009, Basile STARYNKEVITCH wrote:
> > 
> > > > gengtype is presently built only for the build system.  If you want to
> > > > install it you'll need to build it for the host and install that (and,
> > > > yes,
> > > > support --help and --version with the usual adaptation to the
> > > > --with-bugurl
> > > > and --with-pkgversion options).
> > > I have absolutely no idea how to implement all that -and today it is not
> > > my
> > > topmost priority! What would be the code model to follow?
> > 
> > There are plenty of examples in GCC of programs handling --help and
> > --version; if you want something simpler than the gcc driver, you could look
> > at gcov, for example.  Various files used in generator programs and in GCC
> > proper are already built for both host and build systems.
> 
> 
> Sorry, I badly explained why I don't understand.
> 
> What I don't understand is how can gengtype retrieve the exact version string
> of GCC, and the pkgversion. I also am not sure to grasp what exactly

The version numbers would be built into gengtype exactly as they are built 
into GCC, and at the same time, when GCC is built.  The makefile defines 
several macros such as BASEVER when building certain files, in particular 
version.c; I imagine you'd like version.o into the host copy of gengtype.

> -with-pkgversion is for. Does it have something in common with pkg-config
> utility?

http://gcc.gnu.org/wiki/ConfigVersion

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-10-06 14:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-30 21:39 gengtype & plugins [#define emitted in wrong order] Basile STARYNKEVITCH
2009-10-01  6:06 ` Basile STARYNKEVITCH
2009-10-01  6:34   ` Basile STARYNKEVITCH
2009-10-01 19:52   ` Rafael Espindola
2009-10-01 20:33     ` Basile STARYNKEVITCH
2009-10-01 21:02       ` Rafael Espindola
2009-10-02 21:02   ` Gerald Pfeifer
2009-10-03  5:54     ` Basile STARYNKEVITCH
2009-10-05 15:11       ` Diego Novillo
2009-10-05 16:12         ` Basile STARYNKEVITCH
2009-10-05 18:33           ` Rafael Espindola
2009-10-05 19:52           ` Joseph S. Myers
2009-10-06  8:32             ` Basile STARYNKEVITCH
2009-10-06 13:24               ` Joseph S. Myers
2009-10-06 13:52                 ` Basile STARYNKEVITCH
2009-10-06 14:36                   ` Joseph S. Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).