Objective-C/C++ (parsers) : Update @property attribute parsing.

At present, we are missing parsing and checking for around
half of the property attributes in use.  The existing ad hoc scheme
for the parser's communication with the Objective C validation
is not suitable for extending to cover all the missing cases.

Additionally:

1/ We were declaring errors in two cases that the reference
   implementation warns (or is silent).

   I've elected to warn for both those cases, (Wattributes) it
   could be that we should implement Wobjc-xxx-property warning
   masks (TODO).

2/ We were emitting spurious complaints about missing property
   attributes when these were not being parsed because we gave
   up on the first syntax error.

3/ The quality of the diagnostic locations was poor (that's
   true for much of Objective-C, we will have to improve it as
   we modernise areas).

We continue to attempt to keep the code, warning and error output
similar (preferably identical output) between the C and C++ front
ends.

The interface to the Objective-C-specific parts of the parsing is
simplified to a vector of parsed (but not fully-checked) property
attributes, this will simplify the addition of new attributes.

gcc/c-family/ChangeLog:

	* c-objc.h (enum objc_property_attribute_group): New
	(enum objc_property_attribute_kind): New.
	(OBJC_PROPATTR_GROUP_MASK): New.
	(struct property_attribute_info): Small class encapsulating
	parser output from property attributes.
	(objc_prop_attr_kind_for_rid): New
	(objc_add_property_declaration): Simplify interface.
	* stub-objc.c (enum rid): Dummy type.
	(objc_add_property_declaration): Simplify interface.
	(objc_prop_attr_kind_for_rid): New.

gcc/c/ChangeLog:

	* c-parser.c (c_parser_objc_at_property_declaration):
	Improve parsing fidelity. Associate better location info
	with @property attributes.  Clean up the interface to
	objc_add_property_declaration ().

gcc/cp/ChangeLog:

	* parser.c (cp_parser_objc_at_property_declaration):
	Improve parsing fidelity. Associate better location info
	with @property attributes.  Clean up the interface to
	objc_add_property_declaration ().

gcc/objc/ChangeLog:

	* objc-act.c (objc_prop_attr_kind_for_rid): New.
	(objc_add_property_declaration): Adjust to consume the
	parser output using a vector of parsed attributes.

gcc/testsuite/ChangeLog:

	* obj-c++.dg/property/at-property-1.mm: Adjust expected
	diagnostics.
	* obj-c++.dg/property/at-property-29.mm: Likewise.
	* obj-c++.dg/property/at-property-4.mm: Likewise.
	* obj-c++.dg/property/property-neg-2.mm: Likewise.
	* objc.dg/property/at-property-1.m: Likewise.
	* objc.dg/property/at-property-29.m: Likewise.
	* objc.dg/property/at-property-4.m: Likewise.
	* objc.dg/property/at-property-5.m: Likewise.
	* objc.dg/property/property-neg-2.m: Likewise.

1 files changed, 179 insertions, 116 deletions

diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index c0d07ae..26cdedd 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -804,119 +804,74 @@ lookup_property (tree interface_type, tree property)
   return inter;
 }
 
+/* This routine returns a PROPERTY_KIND for the front end RID code supplied.  */
+
+enum objc_property_attribute_kind
+objc_prop_attr_kind_for_rid (enum rid prop_rid)
+{
+  switch (prop_rid)
+    {
+      default:			return OBJC_PROPERTY_ATTR_UNKNOWN;
+      case RID_GETTER:		return OBJC_PROPERTY_ATTR_GETTER;
+      case RID_SETTER:		return OBJC_PROPERTY_ATTR_SETTER;
+
+      case RID_READONLY:	return OBJC_PROPERTY_ATTR_READONLY;
+      case RID_READWRITE:	return OBJC_PROPERTY_ATTR_READWRITE;
+
+      case RID_ASSIGN:		return OBJC_PROPERTY_ATTR_ASSIGN;
+      case RID_RETAIN:		return OBJC_PROPERTY_ATTR_RETAIN;
+      case RID_COPY:		return OBJC_PROPERTY_ATTR_COPY;
+
+      case RID_NONATOMIC:	return OBJC_PROPERTY_ATTR_NONATOMIC;
+
+    }
+}
+
 /* This routine is called by the parser when a
    @property... declaration is found.  'decl' is the declaration of
    the property (type/identifier), and the other arguments represent
    property attributes that may have been specified in the Objective-C
    declaration.  'parsed_property_readonly' is 'true' if the attribute
    'readonly' was specified, and 'false' if not; similarly for the
-   other bool parameters.  'parsed_property_getter_ident' is NULL_TREE
+   other bool parameters.  'property_getter_ident' is NULL_TREE
    if the attribute 'getter' was not specified, and is the identifier
    corresponding to the specified getter if it was; similarly for
-   'parsed_property_setter_ident'.  */
+   'property_setter_ident'.  */
 void
 objc_add_property_declaration (location_t location, tree decl,
-			       bool parsed_property_readonly, bool parsed_property_readwrite,
-			       bool parsed_property_assign, bool parsed_property_retain,
-			       bool parsed_property_copy, bool parsed_property_nonatomic,
-			       tree parsed_property_getter_ident, tree parsed_property_setter_ident)
+			       vec<property_attribute_info *>& prop_attr_list)
 {
-  tree property_decl;
-  tree x;
-  /* 'property_readonly' and 'property_assign_semantics' are the final
-     attributes of the property after all parsed attributes have been
-     considered (eg, if we parsed no 'readonly' and no 'readwrite', ie
-     parsed_property_readonly = false and parsed_property_readwrite =
-     false, then property_readonly will be false because the default
-     is readwrite).  */
-  bool property_readonly = false;
-  objc_property_assign_semantics property_assign_semantics = OBJC_PROPERTY_ASSIGN;
-  bool property_extension_in_class_extension = false;
-
   if (flag_objc1_only)
-    error_at (input_location, "%<@property%> is not available in Objective-C 1.0");
-
-  if (parsed_property_readonly && parsed_property_readwrite)
-    {
-      error_at (location, "%<readonly%> attribute conflicts with %<readwrite%> attribute");
-      /* In case of conflicting attributes (here and below), after
-	 producing an error, we pick one of the attributes and keep
-	 going.  */
-      property_readonly = false;
-    }
-  else
-    {
-      if (parsed_property_readonly)
-	property_readonly = true;
-
-      if (parsed_property_readwrite)
-	property_readonly = false;
-    }
-
-  if (parsed_property_readonly && parsed_property_setter_ident)
-    {
-      error_at (location, "%<readonly%> attribute conflicts with %<setter%> attribute");
-      property_readonly = false;
-    }
-
-  if (parsed_property_assign && parsed_property_retain)
-    {
-      error_at (location, "%<assign%> attribute conflicts with %<retain%> attribute");
-      property_assign_semantics = OBJC_PROPERTY_RETAIN;
-    }
-  else if (parsed_property_assign && parsed_property_copy)
-    {
-      error_at (location, "%<assign%> attribute conflicts with %<copy%> attribute");
-      property_assign_semantics = OBJC_PROPERTY_COPY;
-    }
-  else if (parsed_property_retain && parsed_property_copy)
-    {
-      error_at (location, "%<retain%> attribute conflicts with %<copy%> attribute");
-      property_assign_semantics = OBJC_PROPERTY_COPY;
-    }
-  else
-    {
-      if (parsed_property_assign)
-	property_assign_semantics = OBJC_PROPERTY_ASSIGN;
-
-      if (parsed_property_retain)
-	property_assign_semantics = OBJC_PROPERTY_RETAIN;
-
-      if (parsed_property_copy)
-	property_assign_semantics = OBJC_PROPERTY_COPY;
-    }
+    /* FIXME: we probably ought to bail out at this point.  */
+    error_at (location, "%<@property%> is not available in Objective-C 1.0");
 
+  /* We must be in an interface, category, or protocol.  */
   if (!objc_interface_context)
     {
-      error_at (location, "property declaration not in @interface or @protocol context");
+      error_at (location, "property declaration not in %<@interface%>,"
+			  " %<@protocol%> or %<category%> context");
       return;
     }
 
-  /* At this point we know that we are either in an interface, a
-     category, or a protocol.  */
+  /* Do some spot-checks for the most obvious invalid cases.  */
+
+  gcc_checking_assert (decl && TREE_CODE (decl) == FIELD_DECL);
 
-  /* We expect a FIELD_DECL from the parser.  Make sure we didn't get
-     something else, as that would confuse the checks below.  */
-  if (TREE_CODE (decl) != FIELD_DECL)
+  if (decl && !DECL_NAME (decl))
     {
-      error_at (location, "invalid property declaration");
+      error_at (location, "properties must be named");
       return;
     }
 
-  /* Do some spot-checks for the most obvious invalid types.  */
-
+  location_t decl_loc = DECL_SOURCE_LOCATION (decl);
+  decl_loc = make_location (decl_loc, location, decl_loc);
   if (TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
     {
-      error_at (location, "property cannot be an array");
+      error_at (decl_loc, "property cannot be an array");
       return;
     }
 
-  /* The C++/ObjC++ parser seems to reject the ':' for a bitfield when
-     parsing, while the C/ObjC parser accepts it and gives us a
-     FIELD_DECL with a DECL_INITIAL set.  So we use the DECL_INITIAL
-     to check for a bitfield when doing ObjC.  */
-#ifndef OBJCPLUS
-  if (DECL_INITIAL (decl))
+  if (DECL_C_BIT_FIELD (decl))
     {
       /* A @property is not an actual variable, but it is a way to
 	 describe a pair of accessor methods, so its type (which is
@@ -925,10 +880,110 @@ objc_add_property_declaration (location_t location, tree decl,
 	 and arguments of functions cannot be bitfields).  The
 	 underlying instance variable could be a bitfield, but that is
 	 a different matter.  */
-      error_at (location, "property cannot be a bit-field");
+      error_at (decl_loc, "property cannot be a bit-field");
       return;
     }
-#endif
+
+  /* The final results of parsing the (growing number) of property
+     attributes.  */
+  property_attribute_info *attrs[OBJC_PROPATTR_GROUP_MAX] = { nullptr };
+
+  tree property_getter_ident = NULL_TREE;
+  tree property_setter_ident = NULL_TREE;
+  for (unsigned pn = 0; pn < prop_attr_list.length (); ++pn)
+    {
+      if (prop_attr_list[pn]->parse_error)
+	continue; /* Ignore attributes known to be wrongly parsed.  */
+
+      switch (int g = (int) prop_attr_list[pn]->group())
+	{
+	case OBJC_PROPATTR_GROUP_UNKNOWN:
+	  continue;
+	case OBJC_PROPATTR_GROUP_SETTER:
+	case OBJC_PROPATTR_GROUP_GETTER:
+	  if (attrs[g])
+	    {
+	      warning_at (prop_attr_list[pn]->prop_loc, OPT_Wattributes,
+			  "multiple property %qE methods specified, the latest"
+			  " one will be used", attrs[g]->name);
+	      inform (attrs[g]->prop_loc, "previous specification");
+	    }
+	  attrs[g] = prop_attr_list[pn];
+	  if (g == OBJC_PROPATTR_GROUP_SETTER)
+	    property_setter_ident = attrs[g]->ident;
+	  else
+	    property_getter_ident = attrs[g]->ident;
+	  continue;
+	default:
+	  {
+	    if (!attrs[g])
+	      ;
+	    else if (attrs[g]->prop_kind != prop_attr_list[pn]->prop_kind)
+	      {
+		error_at (prop_attr_list[pn]->prop_loc,
+			  "%qE attribute conflicts with %qE attribute",
+			  prop_attr_list[pn]->name, attrs[g]->name);
+		inform (attrs[g]->prop_loc, "%qE specified here",
+			attrs[g]->name );
+	      }
+	    else
+	      {
+		warning_at (prop_attr_list[pn]->prop_loc, OPT_Wattributes,
+			    "duplicate %qE attribute", attrs[g]->name);
+		inform (attrs[g]->prop_loc, "first specified here");
+	      }
+	    attrs[g] = prop_attr_list[pn];
+	  }
+	  continue;
+	}
+    }
+
+  /* The defaults for atomicity (atomic) and write-ability (readwrite) apply
+     even if the user provides no specified attributes.  */
+  bool property_nonatomic = false;
+  bool property_readonly = false;
+
+  /* Set the values from any specified by the user; these are easy, only two
+     states.  */
+  if (attrs[OBJC_PROPATTR_GROUP_ATOMIC])
+    property_nonatomic = attrs[OBJC_PROPATTR_GROUP_ATOMIC]->prop_kind
+			 == OBJC_PROPERTY_ATTR_NONATOMIC;
+
+  if (attrs[OBJC_PROPATTR_GROUP_READWRITE])
+    property_readonly = attrs[OBJC_PROPATTR_GROUP_READWRITE]->prop_kind
+			 == OBJC_PROPERTY_ATTR_READONLY;
+
+  /* One can't set a readonly value; we issue an error, but force the property
+     to readwrite as well.  */
+  if (property_readonly && property_setter_ident)
+    {
+      error_at (attrs[OBJC_PROPATTR_GROUP_READWRITE]->prop_loc, "%<readonly%>"
+		" attribute conflicts with %<setter%> attribute");
+      gcc_checking_assert (attrs[OBJC_PROPATTR_GROUP_SETTER]);
+      inform (attrs[OBJC_PROPATTR_GROUP_SETTER]->prop_loc, "%<setter%>"
+	      " specified here");
+      property_readonly = false;
+    }
+
+  /* Assign semantics is a tri-state property, and also needs some further
+     checking against the object type.  */
+  objc_property_assign_semantics property_assign_semantics
+    = OBJC_PROPERTY_ASSIGN;
+
+  if (attrs[OBJC_PROPATTR_GROUP_ASSIGN])
+    {
+      if (attrs[OBJC_PROPATTR_GROUP_ASSIGN]->prop_kind
+	  == OBJC_PROPERTY_ATTR_ASSIGN)
+	property_assign_semantics = OBJC_PROPERTY_ASSIGN;
+      else if (attrs[OBJC_PROPATTR_GROUP_ASSIGN]->prop_kind
+	       == OBJC_PROPERTY_ATTR_RETAIN)
+	property_assign_semantics = OBJC_PROPERTY_RETAIN;
+      else if (attrs[OBJC_PROPATTR_GROUP_ASSIGN]->prop_kind
+	       == OBJC_PROPERTY_ATTR_COPY)
+	property_assign_semantics = OBJC_PROPERTY_COPY;
+      else
+	gcc_unreachable ();
+    }
 
   /* TODO: Check that the property type is an Objective-C object or a
      "POD".  */
@@ -950,69 +1005,77 @@ objc_add_property_declaration (location_t location, tree decl,
 	 for non-{Objective-C objects}, and to 'retain' for
 	 Objective-C objects.  But that would break compatibility with
 	 other compilers.  */
-      if (!parsed_property_assign && !parsed_property_retain && !parsed_property_copy)
+      if (!attrs[OBJC_PROPATTR_GROUP_ASSIGN])
 	{
 	  /* Use 'false' so we do not warn for Class objects.  */
 	  if (objc_type_valid_for_messaging (TREE_TYPE (decl), false))
 	    {
-	      warning_at (location,
-			  0,
-			  "object property %qD has no %<assign%>, %<retain%> or %<copy%> attribute; assuming %<assign%>",
-			  decl);
-	      inform (location,
-		      "%<assign%> can be unsafe for Objective-C objects; please state explicitly if you need it");
+	      warning_at (decl_loc, 0, "object property %qD has no %<assign%>,"
+			  " %<retain%> or %<copy%> attribute; assuming"
+			  " %<assign%>", decl);
+	      inform (decl_loc, "%<assign%> can be unsafe for Objective-C"
+		      " objects; please state explicitly if you need it");
 	    }
 	}
     }
 
-  if (property_assign_semantics == OBJC_PROPERTY_RETAIN
-      && !objc_type_valid_for_messaging (TREE_TYPE (decl), true))
-    error_at (location, "%<retain%> attribute is only valid for Objective-C objects");
+  /* Some attributes make no sense unless applied to an Objective-C object.  */
+  bool prop_objc_object_p
+    = objc_type_valid_for_messaging (TREE_TYPE (decl), true);
+  if (!prop_objc_object_p)
+    {
+      tree p_name = NULL_TREE;
+      if (property_assign_semantics == OBJC_PROPERTY_RETAIN
+	  || property_assign_semantics == OBJC_PROPERTY_COPY)
+	p_name = attrs[OBJC_PROPATTR_GROUP_ASSIGN]->name;
 
-  if (property_assign_semantics == OBJC_PROPERTY_COPY
-      && !objc_type_valid_for_messaging (TREE_TYPE (decl), true))
-    error_at (location, "%<copy%> attribute is only valid for Objective-C objects");
+      if (p_name)
+	error_at (decl_loc, "%qE attribute is only valid for Objective-C"
+		  " objects", p_name);
+    }
 
   /* Now determine the final property getter and setter names.  They
      will be stored in the PROPERTY_DECL, from which they'll always be
      extracted and used.  */
 
   /* Adjust, or fill in, setter and getter names.  We overwrite the
-     parsed_property_setter_ident and parsed_property_getter_ident
+     property_setter_ident and property_getter_ident
      with the final setter and getter identifiers that will be
      used.  */
-  if (parsed_property_setter_ident)
+  if (property_setter_ident)
     {
       /* The setter should be terminated by ':', but the parser only
 	 gives us an identifier without ':'.  So, we need to add ':'
 	 at the end.  */
-      const char *parsed_setter = IDENTIFIER_POINTER (parsed_property_setter_ident);
+      const char *parsed_setter = IDENTIFIER_POINTER (property_setter_ident);
       size_t length = strlen (parsed_setter);
       char *final_setter = (char *)alloca (length + 2);
 
       sprintf (final_setter, "%s:", parsed_setter);
-      parsed_property_setter_ident = get_identifier (final_setter);
+      property_setter_ident = get_identifier (final_setter);
     }
   else
     {
       if (!property_readonly)
-	parsed_property_setter_ident = get_identifier (objc_build_property_setter_name
+	property_setter_ident = get_identifier (objc_build_property_setter_name
 						       (DECL_NAME (decl)));
     }
 
-  if (!parsed_property_getter_ident)
-    parsed_property_getter_ident = DECL_NAME (decl);
+  if (!property_getter_ident)
+    property_getter_ident = DECL_NAME (decl);
 
   /* Check for duplicate property declarations.  We first check the
      immediate context for a property with the same name.  Any such
      declarations are an error, unless this is a class extension and
      we are extending a property from readonly to readwrite.  */
+  bool property_extension_in_class_extension = false;
+  tree x = NULL_TREE;
   for (x = CLASS_PROPERTY_DECL (objc_interface_context); x; x = TREE_CHAIN (x))
     {
       if (PROPERTY_NAME (x) == DECL_NAME (decl))
 	{
 	  if (objc_in_class_extension
-	      && property_readonly == 0
+	      && !property_readonly
 	      && PROPERTY_READONLY (x) == 1)
 	    {
 	      /* This is a class extension, and we are extending an
@@ -1087,7 +1150,7 @@ objc_add_property_declaration (location_t location, tree decl,
 	 types, or it is compatible.  */
       location_t original_location = DECL_SOURCE_LOCATION (x);
 
-      if (PROPERTY_NONATOMIC (x) != parsed_property_nonatomic)
+      if (PROPERTY_NONATOMIC (x) != property_nonatomic)
 	{
 	  warning_at (location, 0,
 		      "%<nonatomic%> attribute of property %qD conflicts with "
@@ -1098,7 +1161,7 @@ objc_add_property_declaration (location_t location, tree decl,
 	  return;
 	}
 
-      if (PROPERTY_GETTER_NAME (x) != parsed_property_getter_ident)
+      if (PROPERTY_GETTER_NAME (x) != property_getter_ident)
 	{
 	  warning_at (location, 0,
 		      "%<getter%> attribute of property %qD conflicts with "
@@ -1112,7 +1175,7 @@ objc_add_property_declaration (location_t location, tree decl,
       /* We can only compare the setter names if both the old and new property have a setter.  */
       if (!property_readonly  &&  !PROPERTY_READONLY(x))
 	{
-	  if (PROPERTY_SETTER_NAME (x) != parsed_property_setter_ident)
+	  if (PROPERTY_SETTER_NAME (x) != property_setter_ident)
 	    {
 	      warning_at (location, 0,
 			  "%<setter%> attribute of property %qD conflicts with "
@@ -1190,13 +1253,13 @@ objc_add_property_declaration (location_t location, tree decl,
       if (property_extension_in_class_extension)
 	{
 	  PROPERTY_READONLY (x) = 0;
-	  PROPERTY_SETTER_NAME (x) = parsed_property_setter_ident;
+	  PROPERTY_SETTER_NAME (x) = property_setter_ident;
 	  return;
 	}
     }
 
   /* Create a PROPERTY_DECL node.  */
-  property_decl = make_node (PROPERTY_DECL);
+  tree property_decl = make_node (PROPERTY_DECL);
 
   /* Copy the basic information from the original decl.  */
   TREE_TYPE (property_decl) = TREE_TYPE (decl);
@@ -1205,10 +1268,10 @@ objc_add_property_declaration (location_t location, tree decl,
 
   /* Add property-specific information.  */
   PROPERTY_NAME (property_decl) = DECL_NAME (decl);
-  PROPERTY_GETTER_NAME (property_decl) = parsed_property_getter_ident;
-  PROPERTY_SETTER_NAME (property_decl) = parsed_property_setter_ident;
+  PROPERTY_GETTER_NAME (property_decl) = property_getter_ident;
+  PROPERTY_SETTER_NAME (property_decl) = property_setter_ident;
   PROPERTY_READONLY (property_decl) = property_readonly;
-  PROPERTY_NONATOMIC (property_decl) = parsed_property_nonatomic;
+  PROPERTY_NONATOMIC (property_decl) = property_nonatomic;
   PROPERTY_ASSIGN_SEMANTICS (property_decl) = property_assign_semantics;
   PROPERTY_IVAR_NAME (property_decl) = NULL_TREE;
   PROPERTY_DYNAMIC (property_decl) = 0;