Patchwork Split up init_options langhook

login
register
mail settings
Submitter Eric Botcazou
Date July 1, 2010, 11:31 a.m.
Message ID <201007011331.53244.ebotcazou@adacore.com>
Download mbox | patch
Permalink /patch/57543/
State New
Headers show

Comments

Eric Botcazou - July 1, 2010, 11:31 a.m.
[Sorry for the delay]

> Ada does complicated things with the original argv and argc, which it
> saved in the init_options hook, as well as putting canonical forms of
> various options in gnat_argv/gnat_argc in gnat_handle_option.  I'm not
> clear on exactly what use it makes of each of the arrays save_argv and
> gnat_argv.

gnat_argv is essentially unused, I'll apply the attached patch once the tree 
is open again.

> With the change to init_options no longer getting argc and 
> argv, gnat_init_options now reconstitutes a save_argv array from the
> decoded options it is passed.  I'd urge the Ada maintainers to work
> out how to stop the front end from using save_argv/save_argc at all;
> expecting the front end to replicate exactly the core logic for what
> is an option and what is an argument is very error-prone; whatever it
> needs to do should be done based on logical, decoded options instead
> (option indices and arguments).

Unfortunately the Ada compiler needs to record all the switches passed to it 
since it stores (some of) them in the ALI file.  back_end.adb traverses the 
save_argv array and does some triaging work on it.

> To enable reconstitution of options, a new field in cl_decoded_option
> is added to store a canonical command-line form of an option.  Right
> now this is the original form, but the "canonical" aspect will become
> more relevant in future when support for option aliases is added to
> the option-processing machinery.  This field was planned for when the
> driver started using the common option machinery - the driver needs to
> reconstitute options to process specs - but is being added earlier
> than planned so that options can be reconstituted for Ada, so that the
> langhooks changes can be made without also needing to fix the uses of
> argv made in Ada code.

I think that keeping the original form, or at least providing a backdoor to 
retrieve it, is required for Ada unless things are extensively reworked.

But I'm not a specialist of this part of the compiler so I've CCed Vincent.

Vincent, the thread starts at
  http://gcc.gnu.org/ml/gcc-patches/2010-06/msg01968.html



	* gcc-interface/misc.c (gnat_handle_option): Do not populate gnat_argv.
	(gnat_handle_option): Allocate only one element for gnat_argv.
	(gnat_init): Do not populate gnat_argv.
Joseph S. Myers - July 24, 2010, 8:19 p.m.
On Thu, 1 Jul 2010, Eric Botcazou wrote:

> > To enable reconstitution of options, a new field in cl_decoded_option
> > is added to store a canonical command-line form of an option.  Right
> > now this is the original form, but the "canonical" aspect will become
> > more relevant in future when support for option aliases is added to
> > the option-processing machinery.  This field was planned for when the
> > driver started using the common option machinery - the driver needs to
> > reconstitute options to process specs - but is being added earlier
> > than planned so that options can be reconstituted for Ada, so that the
> > langhooks changes can be made without also needing to fix the uses of
> > argv made in Ada code.
> 
> I think that keeping the original form, or at least providing a backdoor to 
> retrieve it, is required for Ada unless things are extensively reworked.

I believe this patch series has now all been approved so will be 
committing it after retesting.

My basic point is that the concept of "original" forms of options, so far 
as it is meaningful, does not describe something available outside of the 
driver, since the driver already does various rewriting and the options it 
passes to the core compiler may have only very complicated relations to 
those passed to the driver by the user.  It is possible to generate 
textual forms of options from logical option structures if required - and 
it is required for the driver to do this since I don't actually plan to 
make the driver pass binary features to the compilers proper (see Appendix 
2 to my multilibs proposal for why passing such features would logically 
be the right thing to do) - but they should not be assumed to be anything 
other than one of many possible sequences of options equivalent to those 
the user passed.

Patch

Index: gcc-interface/misc.c
===================================================================
--- gcc-interface/misc.c	(revision 161653)
+++ gcc-interface/misc.c	(working copy)
@@ -191,7 +191,6 @@  gnat_handle_option (size_t scode, const
 {
   const struct cl_option *option = &cl_options[scode];
   enum opt_code code = (enum opt_code) scode;
-  char *q;
 
   if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
     {
@@ -201,20 +200,11 @@  gnat_handle_option (size_t scode, const
 
   switch (code)
     {
-    case OPT_I:
-      q = XNEWVEC (char, sizeof("-I") + strlen (arg));
-      strcpy (q, "-I");
-      strcat (q, arg);
-      gnat_argv[gnat_argc] = q;
-      gnat_argc++;
-      break;
-
     case OPT_Wall:
       warn_unused = value;
       warn_uninitialized = value;
       break;
 
-      /* These are used in the GCC Makefile.  */
     case OPT_Wmissing_prototypes:
     case OPT_Wstrict_prototypes:
     case OPT_Wwrite_strings:
@@ -223,15 +213,7 @@  gnat_handle_option (size_t scode, const
     case OPT_Wold_style_definition:
     case OPT_Wmissing_format_attribute:
     case OPT_Woverlength_strings:
-      break;
-
-      /* This is handled by the front-end.  */
-    case OPT_nostdinc:
-      break;
-
-    case OPT_nostdlib:
-      gnat_argv[gnat_argc] = xstrdup ("-nostdlib");
-      gnat_argc++;
+      /* These are used in the GCC Makefile.  */
       break;
 
     case OPT_feliminate_unused_debug_types:
@@ -242,9 +224,8 @@  gnat_handle_option (size_t scode, const
       flag_eliminate_unused_debug_types = -value;
       break;
 
-    case OPT_fRTS_:
-      gnat_argv[gnat_argc] = xstrdup ("-fRTS");
-      gnat_argc++;
+    case OPT_gdwarfplus:
+      gnat_dwarf_extensions = 1;
       break;
 
     case OPT_gant:
@@ -253,22 +234,12 @@  gnat_handle_option (size_t scode, const
       /* ... fall through ... */
 
     case OPT_gnat:
-      /* Recopy the switches without the 'gnat' prefix.  */
-      gnat_argv[gnat_argc] = XNEWVEC (char, strlen (arg) + 2);
-      gnat_argv[gnat_argc][0] = '-';
-      strcpy (gnat_argv[gnat_argc] + 1, arg);
-      gnat_argc++;
-      break;
-
     case OPT_gnatO:
-      gnat_argv[gnat_argc] = xstrdup ("-O");
-      gnat_argc++;
-      gnat_argv[gnat_argc] = xstrdup (arg);
-      gnat_argc++;
-      break;
-
-    case OPT_gdwarfplus:
-      gnat_dwarf_extensions = 1;
+    case OPT_fRTS_:
+    case OPT_I:
+    case OPT_nostdinc:
+    case OPT_nostdlib:
+      /* These are handled by the front-end.  */
       break;
 
     default:
@@ -283,8 +254,7 @@  gnat_handle_option (size_t scode, const
 static unsigned int
 gnat_init_options (unsigned int argc, const char **argv)
 {
-  /* Initialize gnat_argv with save_argv size.  */
-  gnat_argv = (char **) xmalloc ((argc + 1) * sizeof (argv[0]));
+  gnat_argv = (char **) xmalloc (sizeof (argv[0]));
   gnat_argv[0] = xstrdup (argv[0]);     /* name of the command */
   gnat_argc = 1;
 
@@ -423,14 +393,6 @@  gnat_init (void)
   /* Show that REFERENCE_TYPEs are internal and should be Pmode.  */
   internal_reference_types ();
 
-  /* Add the input filename as the last argument.  */
-  if (main_input_filename)
-    {
-      gnat_argv[gnat_argc] = xstrdup (main_input_filename);
-      gnat_argc++;
-      gnat_argv[gnat_argc] = NULL;
-    }
-
   /* Register our internal error function.  */
   global_dc->internal_error = &internal_error_function;