[RFC] New feature to reuse one multilib among different targets

[Joseph S. Myers]

On Tue, 13 Nov 2012, Terry Guo wrote:

> +@findex MULTILIB_REUSE
> +@item MULTILIB_REUSE
> +Sometimes it is desirable to reuse one existing multilib among different
> +targets.  Such kind of reuse can minimize the number of multilib variants.

I don't think "among different targets" is the right wording here.  "for 
different sets of options"?

> +A typical reuse rule is comprised of two parts connected by equality sign.

Not just a typical reuse rule, but *any* reuse rule, as I understand it.

> +The left part of the rule are the options used to build multilib and the right
> +part are the options representing target that will reuse this multilib.  The
> +equality sign in both parts should be replaced with period.

Is the order of the options significant, on either or both sides?  Can the 
options on the right hand side be options that aren't used to build any 
multilibs?  I think the documentation should answer that sort of question.

> @@ -7475,10 +7484,16 @@ set_multilib_dir (void)
>  
>    first = 1;
>    p = multilib_select;
> +
> +  /* Append multilib reuse rules if any.  With those rules, we can reuse
> +     one multilib for certain different targets.  */
> +  if (strlen(multilib_reuse) > 0)

Missing space before '('.

> -      /* Ignore newlines.  */
> -      if (*p == '\n')
> +      /* Ignore newlinesi and spaces.  */

Typo "newlinesi".  And why the change to ignore spaces as well - what's 
different about this new feature to require that?

> @@ -7491,8 +7506,8 @@ set_multilib_dir (void)
>  	  if (*p == '\0')
>  	    {
>  	    invalid_select:
> -	      fatal_error ("multilib select %qs is invalid",
> -			   multilib_select);
> +	      fatal_error ("multilib select %qs%qs is invalid",
> +			   multilib_select, multilib_reuse);

Printing two quoted strings with no space between the closing quote of one 
and the opening quote of the other certainly doesn't seem right.  (Really 
this whole error message seems pretty bad - it won't make sense to users - 
but that's a pre-existing condition.)

> +rm -rf tmpmultilib3
> +cat >tmpmultilib3 <<\EOF

As I understand it, this is a refactoring of existing code.  The patch 
might be easier to review if the bits that just refactored existing code 
into sub-scripts (without any changes to that code) were sent as a 
separate self-contained patch, and then the new feature patch was sent as 
a patch applying on top of those.

> +  # We only care rule that has concrete multilib.

"care about", I think, but this sentence still doesn't really make sense 
to me.  What are the cases that aren't being cared about here, and why are 
they valid inputs?  Surely, given a proper MULTILIB_REUSE setting, every 
rule in that setting should do something meaningful and rules that don't 
should result in errors?

-- 
Joseph S. Myers
joseph@codesourcery.com