[PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Maciej W. Rozycki]

Hi,

 This change makes a LUI complaint message in mips_ip() report the 
value complained about and resemble English a bit more too.

2010-12-02  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (mips_ip)['u']: Report the value of the LUI
	argument complained about; reword the message.

 OK to apply?

  Maciej

binutils-gas-mips-msg-lui.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
@@ -10005,7 +10005,8 @@ mips_ip (char *str, struct mips_cl_insn 
 		  && imm_expr.X_op == O_constant
 		  && (imm_expr.X_add_number < 0
 		      || imm_expr.X_add_number >= 0x10000))
-		as_bad (_("LUI expression not in range 0..65535"));
+		as_bad (_("LUI expression (%lu) not in 0..65535 range"),
+			(unsigned long) imm_expr.X_add_number);
 	      s = expr_end;
 	      continue;
 

Re: [PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Paul Koning]

On Dec 2, 2010, at 2:19 PM, Maciej W. Rozycki wrote:

> Hi,
> 
> This change makes a LUI complaint message in mips_ip() report the 
> value complained about and resemble English a bit more too.
> 
> 2010-12-02  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	gas/
> 	* config/tc-mips.c (mips_ip)['u']: Report the value of the LUI
> 	argument complained about; reword the message.
> 
> OK to apply?
> 
>  Maciej
> 
> binutils-gas-mips-msg-lui.diff
> Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
> @@ -10005,7 +10005,8 @@ mips_ip (char *str, struct mips_cl_insn 
> 		  && imm_expr.X_op == O_constant
> 		  && (imm_expr.X_add_number < 0
> 		      || imm_expr.X_add_number >= 0x10000))
> -		as_bad (_("LUI expression not in range 0..65535"));
> +		as_bad (_("LUI expression (%lu) not in 0..65535 range"),
> +			(unsigned long) imm_expr.X_add_number);
> 	      s = expr_end;
> 	      continue;
> 
I'm not a native speaker of English, but the original word order (...in range x..y) seems better to me.

	paul

Re: [PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Maciej W. Rozycki]

On Thu, 2 Dec 2010, Paul Koning wrote:

> > @@ -10005,7 +10005,8 @@ mips_ip (char *str, struct mips_cl_insn 
> > 		  && imm_expr.X_op == O_constant
> > 		  && (imm_expr.X_add_number < 0
> > 		      || imm_expr.X_add_number >= 0x10000))
> > -		as_bad (_("LUI expression not in range 0..65535"));
> > +		as_bad (_("LUI expression (%lu) not in 0..65535 range"),
> > +			(unsigned long) imm_expr.X_add_number);
> > 	      s = expr_end;
> > 	      continue;
> > 
> I'm not a native speaker of English, but the original word order (...in 
> range x..y) seems better to me.

 I have changed the order so that "0..65535" precedes the noun it 
describes as an adjective (substitute e.g. "(the) correct" for 
comparison); alternatively "in (the) range of 0..65535" where "0..65535" 
is used as a noun would seem correct to me as well.

 Anyway the mention of the problematic value is what matters here and I 
won't insist on the rest of the change.

 While at it I'd like to rise my concern about the lack of a way to output 
numbers of the offsetT or addressT type on hosts where these boil down to 
the long long type -- I realise in the old days it used to be problematic, 
but nowadays we have <inttypes.h> with its PRI* macros, so perhaps we 
should factor them in one way or another.  Otherwise we risk confusing the 
user here or there when values with some high-order bits get truncated and 
error messages like this report values that might appear valid once that 
happened.

  Maciej

Re: [PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Hans-Peter Nilsson]

On Thu, 2 Dec 2010, Maciej W. Rozycki wrote:
> 2010-12-02  Maciej W. Rozycki  <macro@codesourcery.com>
>
> 	gas/
> 	* config/tc-mips.c (mips_ip)['u']: Report the value of the LUI
> 	argument complained about; reword the message.

Just a bikeshed comment from the sideline: The [] in changelog-
entries is canonically for conditionally defined stuff; within
#ifdef X/#if X.  For case statements, the canonical marking is
<case X> or <X>, like:

	* config/tc-mips.c (mips_ip) <case 'u'>: Report the value of the LUI
or
	* config/tc-mips.c (mips_ip) <'u'>: Report the value of the LUI

Feel free to ignore this, I just don't want your format to catch
on - and possibly be forced to use it. ;)

brgds, H-P

Re: [PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Richard Sandiford]

Patches 4-7 OK

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-01 21:05:50.000000000 +0000
> @@ -10005,7 +10005,8 @@ mips_ip (char *str, struct mips_cl_insn 
>  		  && imm_expr.X_op == O_constant
>  		  && (imm_expr.X_add_number < 0
>  		      || imm_expr.X_add_number >= 0x10000))
> -		as_bad (_("LUI expression not in range 0..65535"));
> +		as_bad (_("LUI expression (%lu) not in 0..65535 range"),
> +			(unsigned long) imm_expr.X_add_number);
>  	      s = expr_end;
>  	      continue;

Like Paul, I think the first order ("expression is not in the range A..B")
sounds more natural.  There again, the liberal dropping of words makes
the thing sound wrong whatever we do.

Patch is OK with the original word order.

Richard

Re: [PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Maciej W. Rozycki]

On Tue, 7 Dec 2010, Richard Sandiford wrote:

> Like Paul, I think the first order ("expression is not in the range A..B")
> sounds more natural.  There again, the liberal dropping of words makes
> the thing sound wrong whatever we do.
> 
> Patch is OK with the original word order.

 Can't argue with a native English speaker. ;)  This is what I applied, 
thanks.

2010-12-09  Maciej W. Rozycki  <macro@codesourcery.com>

	* config/tc-mips.c (mips_ip) <'u'>: Report the value of the LUI
	argument complained about; reword the message.

  Maciej

binutils-gas-mips-msg-lui.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-12-09 02:34:52.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-12-09 02:34:53.000000000 +0000
@@ -10014,7 +10014,8 @@ mips_ip (char *str, struct mips_cl_insn 
 		  && imm_expr.X_op == O_constant
 		  && (imm_expr.X_add_number < 0
 		      || imm_expr.X_add_number >= 0x10000))
-		as_bad (_("lui expression not in range 0..65535"));
+		as_bad (_("lui expression (%lu) not in range 0..65535"),
+			(unsigned long) imm_expr.X_add_number);
 	      s = expr_end;
 	      continue;
 

Re: [PATCH 08/20] MIPS/GAS: Improve a LUI complaint message

[Maciej W. Rozycki]

On Sun, 5 Dec 2010, Hans-Peter Nilsson wrote:

> Just a bikeshed comment from the sideline: The [] in changelog-
> entries is canonically for conditionally defined stuff; within
> #ifdef X/#if X.  For case statements, the canonical marking is
> <case X> or <X>, like:
> 
> 	* config/tc-mips.c (mips_ip) <case 'u'>: Report the value of the LUI
> or
> 	* config/tc-mips.c (mips_ip) <'u'>: Report the value of the LUI
> 
> Feel free to ignore this, I just don't want your format to catch
> on - and possibly be forced to use it. ;)

 Sorry for the late response -- thanks for the hint and as you may have 
noticed I did take it into account.  I feel no reason to diverge from the 
established practice merely because I missed some of its details.

  Maciej