Patchwork =?UTF-8?Q?Fix=20for=20PR=20objc/47232=20("Confusing=20ObjC=20error=20mes?= =?UTF-8?Q?sage=20"attributes=20may=20not=20be=20specified=20before=20befo?= =?UTF-8?Q?re=20=E2=80=98class=E2=80=99")?=

login
register
mail settings
Submitter Nicola Pero
Date Jan. 9, 2011, 2:56 p.m.
Message ID <1294584963.498131375@192.168.4.58>
Download mbox | patch
Permalink /patch/78022/
State New
Headers show

Comments

Nicola Pero - Jan. 9, 2011, 2:56 p.m.
This patch fixes PR objc/47232.

The problem is simply the ObjC error message (new in 4.6)

  error: attributes may not be specified before before ‘class’

which is hard to read and jumps out as convoluted if not wrong because of the 
repeated 'before'.  This patch changes it to read

  error: unexpected attribute before 'class'

which is consistent with the rest of the parser errors, and is more readable
(if anyone has better suggestions, let me know!). ;-)

Ok to commit ?

Thanks
IainS - Jan. 9, 2011, 3:55 p.m.
Hi Nicola,

On 9 Jan 2011, at 14:56, Nicola Pero wrote:

> This patch fixes PR objc/47232.
>
> The problem is simply the ObjC error message (new in 4.6)
>
>  error: attributes may not be specified before before ‘class’

oops... ;-)

the idea was to convey that attributes are not permitted in that  
position ...
...  'may*** not be specified'...

(since there is scope for confusion in the programmer's mind - because  
prefix attributes are required elsewhere).

> which is hard to read and jumps out as convoluted if not wrong  
> because of the
> repeated 'before'.  This patch changes it to read
>
>  error: unexpected attribute before 'class'
>
> which is consistent with the rest of the parser errors, and is more  
> readable
> (if anyone has better suggestions, let me know!). ;-)

attributes not permitted ?
I'd prefer a proper sentence... but...
Your proposition is also OK ;-)

Iain

***
perhaps en_US is grammatically different in this respect -- since I  
see things like "x may be used un-initialized in... "
(which implies that it is permitted to use it in such a position in  
en_GB)
whereas... we know it really means "x might be used un-initialized  
in... "

>
> Ok to commit ?
>
> Thanks
>
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog   (revision 168611)
> +++ ChangeLog   (working copy)
> @@ -1,3 +1,9 @@
> +2011-01-09  Nicola Pero  <nicola.pero@meta-innovation.com>
> +
> +       PR objc/47232
> +       * c-parser.c (c_parser_declaration_or_fndef): Improved
> +       error message.
> +
> 2011-01-09  Iain Sandoe  <iains@gcc.gnu.org>
>
>        PR gcc/46902
> Index: testsuite/ChangeLog
> ===================================================================
> --- testsuite/ChangeLog (revision 168611)
> +++ testsuite/ChangeLog (working copy)
> @@ -1,3 +1,9 @@
> +2011-01-09  Nicola Pero  <nicola.pero@meta-innovation.com>
> +
> +       PR objc/47232
> +       * objc.dg/attributes/invalid-attribute-1.m: New.
> +       * obj-c++.dg/attributes/invalid-attribute-1.mm: New.
> +
> 2011-01-09  Janus Weil  <janus@gcc.gnu.org>
>
>        PR fortran/46313
> Index: testsuite/objc.dg/attributes/invalid-attribute-1.m
> ===================================================================
> --- testsuite/objc.dg/attributes/invalid-attribute-1.m  (revision 0)
> +++ testsuite/objc.dg/attributes/invalid-attribute-1.m  (revision 0)
> @@ -0,0 +1,6 @@
> +/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>,  
> January 2011.  */
> +/* { dg-do compile } */
> +
> +#include <objc/objc.h>
> +
> +__attribute__ ((deprecated)) @class A; /* { dg-error "unexpected  
> attribute before .class." } */
> Index: testsuite/obj-c++.dg/attributes/invalid-attribute-1.mm
> ===================================================================
> --- testsuite/obj-c++.dg/attributes/invalid-attribute-1.mm       
> (revision 0)
> +++ testsuite/obj-c++.dg/attributes/invalid-attribute-1.mm       
> (revision 0)
> @@ -0,0 +1,6 @@
> +/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>,  
> January 2011.  */
> +/* { dg-do compile } */
> +
> +#include <objc/objc.h>
> +
> +__attribute__ ((deprecated)) @class A; /* { dg-error "attributes  
> may not be specified before the ..class. Objective-C.. keyword" } */
> Index: c-parser.c
> ===================================================================
> --- c-parser.c  (revision 168611)
> +++ c-parser.c  (working copy)
> @@ -1555,8 +1555,7 @@ c_parser_declaration_or_fndef (c_parser
>        case RID_AT_PROPERTY:
>          if (specs->attrs)
>            {
> -             c_parser_error (parser,
> -                             "attributes may not be specified  
> before" );
> +             c_parser_error (parser, "unexpected attribute");
>              specs->attrs = NULL;
>            }
>          break;
>
>
Nicola Pero - Jan. 9, 2011, 4:19 p.m.
>> which is hard to read and jumps out as convoluted if not wrong  
>> because of the
>> repeated 'before'.  This patch changes it to read
>>
>> error: unexpected attribute before 'class'
>>
>> which is consistent with the rest of the parser errors, and is more  
>> readable
>> (if anyone has better suggestions, let me know!). ;-)
>
> attributes not permitted ?
> I'd prefer a proper sentence... but...
> Your proposition is also OK ;-)

I proposed 'unexpected attribute' because it is consistent with the  
other errors, such as 'expected identifier', 'unknown type name' etc.
It also works no matter if you add "before 'class'" after it or not. ;-)

But I actually like sentences, like you do, and agree that a more  
explanatory sentence would be better.  But maybe all error messages
should be changed in sync then ?

Thanks
Mike Stump - Jan. 9, 2011, 7:04 p.m.
On Jan 9, 2011, at 6:56 AM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> This patch fixes PR objc/47232.
> 
> The problem is simply the ObjC error message (new in 4.6)
> 
>  error: attributes may not be specified before

> Ok to commit ?

Ok.

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 168611)
+++ ChangeLog   (working copy)
@@ -1,3 +1,9 @@ 
+2011-01-09  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/47232
+       * c-parser.c (c_parser_declaration_or_fndef): Improved
+       error message.
+
 2011-01-09  Iain Sandoe  <iains@gcc.gnu.org>
 
        PR gcc/46902
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 168611)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,9 @@ 
+2011-01-09  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/47232
+       * objc.dg/attributes/invalid-attribute-1.m: New.
+       * obj-c++.dg/attributes/invalid-attribute-1.mm: New.    
+
 2011-01-09  Janus Weil  <janus@gcc.gnu.org>
 
        PR fortran/46313
Index: testsuite/objc.dg/attributes/invalid-attribute-1.m
===================================================================
--- testsuite/objc.dg/attributes/invalid-attribute-1.m  (revision 0)
+++ testsuite/objc.dg/attributes/invalid-attribute-1.m  (revision 0)
@@ -0,0 +1,6 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, January 2011.  */
+/* { dg-do compile } */
+
+#include <objc/objc.h>
+
+__attribute__ ((deprecated)) @class A; /* { dg-error "unexpected attribute before .class." } */
Index: testsuite/obj-c++.dg/attributes/invalid-attribute-1.mm
===================================================================
--- testsuite/obj-c++.dg/attributes/invalid-attribute-1.mm      (revision 0)
+++ testsuite/obj-c++.dg/attributes/invalid-attribute-1.mm      (revision 0)
@@ -0,0 +1,6 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, January 2011.  */
+/* { dg-do compile } */
+
+#include <objc/objc.h>
+
+__attribute__ ((deprecated)) @class A; /* { dg-error "attributes may not be specified before the ..class. Objective-C.. keyword" } */
Index: c-parser.c
===================================================================
--- c-parser.c  (revision 168611)
+++ c-parser.c  (working copy)
@@ -1555,8 +1555,7 @@  c_parser_declaration_or_fndef (c_parser 
        case RID_AT_PROPERTY:
          if (specs->attrs)
            {
-             c_parser_error (parser, 
-                             "attributes may not be specified before" );
+             c_parser_error (parser, "unexpected attribute");
              specs->attrs = NULL;
            }
          break;