public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Don’t insert white space in ‘orig_option_with_args_text’ for OPT_l
@ 2012-03-09 15:38 Ludovic Courtès
  2012-03-09 17:28 ` [PATCH] Don't insert white space in 'orig_option_with_args_text' " Joseph S. Myers
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2012-03-09 15:38 UTC (permalink / raw)
  To: gcc-patches


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

Hi,

This patch changes ‘generate_option’ to not insert any white spaces
after ‘-l’.

This fixes a problem introduced in GCC 4.6 (r163459) whereby
‘gfortran -v’ would emit erroneous strings like this:

  Driving: […] -l gfortran -l m -shared-libgcc […]

Note the space after ‘-l’.

(In turn, that would confuse Libtool’s ‘_LT_SYS_HIDDEN_LIBDEPS’ macro,
which would determine something like this:

  postdeps="[…] -L/nix/store/wnzgsfhmb3ys5ssfgpcpwjnmdzn717mk-gfortran-4.6.3/lib -l -l […]"

eventually leading to a link command-line like this:

  libtool: link: gfortran -shared […] -l -L/nix/store/blsdhiik2lk4zmz3hbzf77g6hcrq7ckx-gfortran-wrapper-4.6.3/bin […]

leading to an error like:

  ld: cannot find -l-L/nix/store/blsdhiik2lk4zmz3hbzf77g6hcrq7ckx-gfortran-wrapper-4.6.3/bin

Ouch!)

Thanks,
Ludo’.

2012-03-09  Ludovic Courtès  <ludovic.courtes@inria.fr>

	* gcc/opts-common.c (generate_option): Don't insert white space in
	`canonical_option' when OPT_INDEX is OPT_l.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Type: text/x-patch, Size: 630 bytes --]

diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 354bce0..60ec02d 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -931,6 +931,12 @@ generate_option (size_t opt_index, const char *arg, int value,
       break;
 
     case 2:
+      if (opt_index == OPT_l)
+	/* Don't insert any white spaces between `-l' and its argument.  */
+	decoded->orig_option_with_args_text
+	  = concat (decoded->canonical_option[0],
+		    decoded->canonical_option[1], NULL);
+      else
       decoded->orig_option_with_args_text
 	= concat (decoded->canonical_option[0], " ",
 		  decoded->canonical_option[1], NULL);

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: [PATCH] Don't insert white space in 'orig_option_with_args_text' for OPT_l
  2012-03-09 15:38 [PATCH] Don’t insert white space in ‘orig_option_with_args_text’ for OPT_l Ludovic Courtès
@ 2012-03-09 17:28 ` Joseph S. Myers
  2012-03-12 11:02   ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Joseph S. Myers @ 2012-03-09 17:28 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: gcc-patches

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 705 bytes --]

On Fri, 9 Mar 2012, Ludovic Courtès wrote:

> 	* gcc/opts-common.c (generate_option): Don't insert white space in
> 	`canonical_option' when OPT_INDEX is OPT_l.

No, opts-common.c should not have special cases for individual options 
like that.  The canonical form has the separate arguments.  gcc.c has a 
special case for how it passes this option to subprocesses; if you have 
problems with some output from gfortran involving -l options, you should 
change the gfortran driver as needed so it outputs -l options in a 
different way.

Your subject refers to orig_option_with_args_text while your ChangeLog 
entry refers to canonical_option.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH] Don't insert white space in 'orig_option_with_args_text' for OPT_l
  2012-03-09 17:28 ` [PATCH] Don't insert white space in 'orig_option_with_args_text' " Joseph S. Myers
@ 2012-03-12 11:02   ` Ludovic Courtès
  2012-03-12 12:41     ` Joseph S. Myers
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2012-03-12 11:02 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: gcc-patches

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

Hi,

Thanks for the review.

"Joseph S. Myers" <joseph@codesourcery.com> skribis:

> On Fri, 9 Mar 2012, Ludovic Courtès wrote:
>
>> 	* gcc/opts-common.c (generate_option): Don't insert white space in
>> 	`canonical_option' when OPT_INDEX is OPT_l.
>
> No, opts-common.c should not have special cases for individual options 
> like that.  The canonical form has the separate arguments.

Indeed, and the comment in opts.h makes it clear, though I think I was
confused by the phrase “original text of option”.

> gcc.c has a special case for how it passes this option to
> subprocesses; if you have problems with some output from gfortran
> involving -l options, you should change the gfortran driver as needed
> so it outputs -l options in a different way.

The patch below solves the problem in a gfortran-specific way.  WDYT?

Thanks,
Ludo’.

2012-03-09  Ludovic Courtès  <ludovic.courtes@inria.fr>

	* gcc/fortran/gfotranspec.c (lang_specific_driver): When
	VERBOSE, make sure `-l' options are printed with no intertwined
	white spaces.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 698 bytes --]

diff --git a/gcc/fortran/gfortranspec.c b/gcc/fortran/gfortranspec.c
index 2240bfb..55e5e42 100644
--- a/gcc/fortran/gfortranspec.c
+++ b/gcc/fortran/gfortranspec.c
@@ -461,8 +461,15 @@ For more information about these matters, see the file named COPYING\n\n"));
     {
       fprintf (stderr, _("Driving:"));
       for (i = 0; i < g77_newargc; i++)
+	{
+	  if (g77_new_decoded_options[i].opt_index == OPT_l)
+	    /* Make sure no white space is inserted after `-l'.  */
+	    fprintf (stderr, " -l%s",
+		     g77_new_decoded_options[i].canonical_option[1]);
+	  else
 	fprintf (stderr, " %s",
 		 g77_new_decoded_options[i].orig_option_with_args_text);
+	}
       fprintf (stderr, "\n");
     }

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

* Re: [PATCH] Don't insert white space in 'orig_option_with_args_text' for OPT_l
  2012-03-12 11:02   ` Ludovic Courtès
@ 2012-03-12 12:41     ` Joseph S. Myers
  2012-03-13 10:12       ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Joseph S. Myers @ 2012-03-12 12:41 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: gcc-patches

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 220 bytes --]

On Mon, 12 Mar 2012, Ludovic Courtès wrote:

> The patch below solves the problem in a gfortran-specific way.  WDYT?

I think that's the right approach for this issue.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH] Don't insert white space in 'orig_option_with_args_text' for OPT_l
  2012-03-12 12:41     ` Joseph S. Myers
@ 2012-03-13 10:12       ` Ludovic Courtès
  2012-03-29  8:38         ` [PATCH, ping #1] " Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2012-03-13 10:12 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: gcc-patches, Paul Brook

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

Hi,

(Cc: Paul Brook.)

"Joseph S. Myers" <joseph@codesourcery.com> skribis:

> On Mon, 12 Mar 2012, Ludovic Courtès wrote:
>
>> The patch below solves the problem in a gfortran-specific way.  WDYT?
>
> I think that's the right approach for this issue.

The previous patch was produced with ‘diff -b’.  Here’s a fixed one.
Let me know if anything else needs to be done.

Thanks,
Ludo’.

2012-03-09  Ludovic Courtès  <ludovic.courtes@inria.fr>

	* gcc/fortran/gfotranspec.c (lang_specific_driver): When
	VERBOSE, make sure `-l' options are printed with no intertwined
	white spaces.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 792 bytes --]

diff --git a/gcc/fortran/gfortranspec.c b/gcc/fortran/gfortranspec.c
index 2240bfb..55e5e42 100644
--- a/gcc/fortran/gfortranspec.c
+++ b/gcc/fortran/gfortranspec.c
@@ -461,8 +461,15 @@ For more information about these matters, see the file named COPYING\n\n"));
     {
       fprintf (stderr, _("Driving:"));
       for (i = 0; i < g77_newargc; i++)
-	fprintf (stderr, " %s",
-		 g77_new_decoded_options[i].orig_option_with_args_text);
+	{
+	  if (g77_new_decoded_options[i].opt_index == OPT_l)
+	    /* Make sure no white space is inserted after `-l'.  */
+	    fprintf (stderr, " -l%s",
+		     g77_new_decoded_options[i].canonical_option[1]);
+	  else
+	    fprintf (stderr, " %s",
+		     g77_new_decoded_options[i].orig_option_with_args_text);
+	}
       fprintf (stderr, "\n");
     }

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

* [PATCH, ping #1] Don't insert white space in 'orig_option_with_args_text' for OPT_l
  2012-03-13 10:12       ` Ludovic Courtès
@ 2012-03-29  8:38         ` Ludovic Courtès
  0 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2012-03-29  8:38 UTC (permalink / raw)
  To: Paul Brook; +Cc: gcc-patches, Joseph S. Myers

Ping.

ludovic.courtes@inria.fr (Ludovic Courtès) skribis:

> "Joseph S. Myers" <joseph@codesourcery.com> skribis:
>
>> On Mon, 12 Mar 2012, Ludovic Courtès wrote:
>>
>>> The patch below solves the problem in a gfortran-specific way.  WDYT?
>>
>> I think that's the right approach for this issue.
>
> The previous patch was produced with ‘diff -b’.  Here’s a fixed one.
> Let me know if anything else needs to be done.
>
> Thanks,
> Ludo’.
>
> 2012-03-09  Ludovic Courtès  <ludovic.courtes@inria.fr>
>
> 	* gcc/fortran/gfotranspec.c (lang_specific_driver): When
> 	VERBOSE, make sure `-l' options are printed with no intertwined
> 	white spaces.
>
> diff --git a/gcc/fortran/gfortranspec.c b/gcc/fortran/gfortranspec.c
> index 2240bfb..55e5e42 100644
> --- a/gcc/fortran/gfortranspec.c
> +++ b/gcc/fortran/gfortranspec.c
> @@ -461,8 +461,15 @@ For more information about these matters, see the file named COPYING\n\n"));
>      {
>        fprintf (stderr, _("Driving:"));
>        for (i = 0; i < g77_newargc; i++)
> -	fprintf (stderr, " %s",
> -		 g77_new_decoded_options[i].orig_option_with_args_text);
> +	{
> +	  if (g77_new_decoded_options[i].opt_index == OPT_l)
> +	    /* Make sure no white space is inserted after `-l'.  */
> +	    fprintf (stderr, " -l%s",
> +		     g77_new_decoded_options[i].canonical_option[1]);
> +	  else
> +	    fprintf (stderr, " %s",
> +		     g77_new_decoded_options[i].orig_option_with_args_text);
> +	}
>        fprintf (stderr, "\n");
>      }

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

end of thread, other threads:[~2012-03-29  8:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-09 15:38 [PATCH] Don’t insert white space in ‘orig_option_with_args_text’ for OPT_l Ludovic Courtès
2012-03-09 17:28 ` [PATCH] Don't insert white space in 'orig_option_with_args_text' " Joseph S. Myers
2012-03-12 11:02   ` Ludovic Courtès
2012-03-12 12:41     ` Joseph S. Myers
2012-03-13 10:12       ` Ludovic Courtès
2012-03-29  8:38         ` [PATCH, ping #1] " Ludovic Courtès

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).