diff mbox

=?UTF-8?Q?ObjC/ObjC++:=20improve=20error=20message=20for=20missing=20'?= =?UTF-8?Q?=3D'=20after=20'setter'=20or=20'getter'=20property=20attribute?=

Message ID 1294784310.37871646@192.168.4.58
State New
Headers show

Commit Message

Nicola Pero Jan. 11, 2011, 10:18 p.m. UTC 
This (trivial) patch improves on the error messages

(ObjC)
 at-property-29.m:11:18: error: getter/setter attribute must be followed by ‘=’ before ‘)’ token

(ObjC++)
 at-property-29.mm:11:18: error: getter/setter/ivar attribute must be followed by ‘=’ before ‘)’ token

The ObjC++ message still mentioned the 'ivar' attribute which no longer exist.  Both the ObjC and ObjC++
messages were vague; the compiler knows whether the error is caused by a getter attribute
or a setter attribute (you often have both in the same @property declaration), but it didn't bother
clearly stating which one.   Finally, there were no testcases.

This patch fixes the problems and replaces these messages with errors such as:

 at-property-29.m:11:18: error: missing ‘=’ (after ‘getter’ attribute) before ‘)’ token
 at-property-29.mm:12:18: error: missing ‘=’ (after ‘setter’ attribute) before ‘)’ token

(which hopefully are very clear and point you directly to exactly where the problem is) and adds
the testcases.

Ok to commit ?

Thanks

PS: Note that I also added a FIXME in the ObjC++ parsing code as I found another
very very minor ObjC++ parser bug while looking at this (ie, a spurious, additional
error message would be produced by '@property (blablabla, retain) int x;' in ObjC++),
but don't really want to make arbitrary changes at this stage so for now I am content
with only adding a FIXME in there.

Comments

Mike Stump Jan. 12, 2011, 12:30 a.m. UTC | #1 
On Jan 11, 2011, at 2:18 PM, Nicola Pero wrote:
> This (trivial) patch improves on the error messages

> Ok to commit ?

Ok.
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 168679)
+++ ChangeLog   (working copy)
@@ -1,3 +1,13 @@ 
+2011-01-11  Nicola Pero  <nicola.pero@meta-innovation.com>     
+
+       * c-parser.c (c_parser_objc_at_property_declaration): Improved
+       error message.  
+       
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 168679)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@ 
+2011-01-11  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/property/at-property-29.m: New.
+       * obj-c++.dg/property/at-property-29.mm: New.   
+       
 2011-01-11  Richard Henderson  <rth@redhat.com>
 
        * gcc-dg/tree-ssa/vrp47.c: Disable for mn10300 as well.
Index: testsuite/objc.dg/property/at-property-29.m
===================================================================
--- testsuite/objc.dg/property/at-property-29.m (revision 0)
+++ testsuite/objc.dg/property/at-property-29.m (revision 0)
@@ -0,0 +1,14 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, January 2011.  */
+/* { dg-do compile } */
+
+#include <objc/objc.h>
+
+@interface MyRootClass
+{
+  Class isa;
+}
+/* Test missing '=' in setter/getter attributes.  */
+@property (getter)  int property_a; /* { dg-error "missing .=. .after .getter. attribute." } */
+@property (setter) int property_b;  /* { dg-error "missing .=. .after .setter. attribute." } */
+@property (assign, getter) int property_c; /* { dg-error "missing .=. .after .getter. attribute." } */
+@end
Index: testsuite/obj-c++.dg/property/at-property-29.mm
===================================================================
--- testsuite/obj-c++.dg/property/at-property-29.mm     (revision 0)
+++ testsuite/obj-c++.dg/property/at-property-29.mm     (revision 0)
@@ -0,0 +1,14 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, January 2011.  */
+/* { dg-do compile } */
+
+#include <objc/objc.h>
+
+@interface MyRootClass
+{
+  Class isa;
+}
+/* Test missing '=' in setter/getter attributes.  */
+@property (getter)  int property_a; /* { dg-error "missing .=. .after .getter. attribute." } */
+@property (setter) int property_b;  /* { dg-error "missing .=. .after .setter. attribute." } */
+@property (assign, getter) int property_c; /* { dg-error "missing .=. .after .getter. attribute." } */
+@end
Index: cp/ChangeLog
===================================================================
--- cp/ChangeLog        (revision 168679)
+++ cp/ChangeLog        (working copy)
@@ -1,3 +1,8 @@ 
+2011-01-11  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * parser.c (cp_parser_objc_at_property_declaration): Improved
+       error message.
+
 2011-01-11  Jason Merrill  <jason@redhat.com>
 
        PR c++/46658
Index: cp/parser.c
===================================================================
--- cp/parser.c (revision 168679)
+++ cp/parser.c (working copy)
@@ -23087,8 +23087,12 @@  cp_parser_objc_at_property_declaration (cp_parser
            case RID_SETTER:
              if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ))
                {
-                 cp_parser_error (parser,
-                                  "getter/setter/ivar attribute must be followed by %<=%>");
+                 if (keyword == RID_GETTER)
+                   cp_parser_error (parser,
+                                    "missing %<=%> (after %<getter%> attribute)");
+                 else
+                   cp_parser_error (parser,
+                                    "missing %<=%> (after %<setter%> attribute)");
                  syntax_error = true;
                  break;
                }
@@ -23128,13 +23132,17 @@  cp_parser_objc_at_property_declaration (cp_parser
 
          if (syntax_error)
            break;
-         
+
          if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA))
            cp_lexer_consume_token (parser->lexer);
          else
            break;
        }
 
+      /* FIXME: "@property (setter, assign);" will generate a spurious
+        "error: expected ‘)’ before ‘,’ token".  This is because
+        cp_parser_require, unlike the C counterpart, will produce an
+        error even if we are in error recovery.  */
       if (!cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN))
        {
          cp_parser_skip_to_closing_parenthesis (parser,
Index: c-parser.c
===================================================================
--- c-parser.c  (revision 168679)
+++ c-parser.c  (working copy)
@@ -7961,8 +7957,12 @@  c_parser_objc_at_property_declaration (c_parser *p
            case RID_SETTER:
              if (c_parser_next_token_is_not (parser, CPP_EQ))
                {
-                 c_parser_error (parser,
-                                 "getter/setter attribute must be followed by %<=%>");
+                 if (keyword == RID_GETTER)
+                   c_parser_error (parser,
+                                   "missing %<=%> (after %<getter%> attribute)");
+                 else
+                   c_parser_error (parser,
+                                   "missing %<=%> (after %<setter%> attribute)");
                  syntax_error = true;
                  break;
                }