ping^2 Re: [PATCH] Objective-C++ : Fix handling of unnamed message parms [PR49070].

[Iain Sandoe]

Iain Sandoe <iain@sandoe.co.uk> wrote:

> Although this is an Objective-C++ patch, I need to touch stuff outside  
> “objc local”
> contexts, so think it needs review,
>
> (this is a regression fix for all open branches).
>
> Iain Sandoe <iain@sandoe.co.uk> wrote:
>
>> We were discussing proposed reflection splicing syntax - [:reflection:]  
>> in SG7
>> which has some similarities with objective-c++ message syntax with unnamed
>> params.
>>
>> This reminded me that we have a *very* long-standing regression against
>> objective-c++ where it is not able to handle unnamed message parameters  
>> (an
>> idiom used in practice).
>>
>> Fixed as below.
>> tested on x86_64-darwin, x86_64-linux-gnu,
>> OK for master / open branches?
>>
>> thanks
>> Iain
>>
>> ------
>>
>> When we are parsing an Objective-C++ message, a colon is a valid
>> terminator for a assignment-expression.  That is:
>>
>> [receiver meth:x:x:x:x];
>>
>> Is a valid, if somewhat unreadable, construction; corresponding
>> to a method declaration like:
>>
>> - (id) meth:(id)arg0 :(id)arg1 :(id)arg2 :(id)arg3;
>>
>> Where three of the message params have no name.
>>
>> If fact, although it might be unintentional, Objective-C/C++ can
>> accept message selectors with all the parms unnamed (this applies
>> to the clang implementation too, which is taken as the reference
>> for the language).
>>
>> For regular C++, the pattern x:x is not valid in that position an
>> an error is emitted with a fixit for the expected scope token.
>>
>> If we simply made that error conditional on !c_c_dialect_objc()
>> that would regress Objective-C++ diagnostics for cases outside a
>> message selector, so we add a state flag for this.
>>
>> gcc/cp/ChangeLog:
>>
>> 	PR objc++/49070
>> 	* parser.c (cp_debug_parser): Add Objective-C++ message
>> 	state flag.
>> 	(cp_parser_nested_name_specifier_opt): Allow colon to
>> 	terminate an assignment-expression when parsing Objective-
>> 	C++ messages.
>> 	(cp_parser_objc_message_expression): Set and clear message
>> 	parsing state on entry and exit.
>> 	* parser.h (struct cp_parser): Add a context flag for
>> 	Objective-C++ message state.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR objc++/49070
>> 	* obj-c++.dg/pr49070.mm: New test.
>> 	* objc.dg/unnamed-parms.m: New test.
>> ---
>> gcc/cp/parser.c                       |  8 ++++-
>> gcc/cp/parser.h                       |  4 +++
>> gcc/testsuite/obj-c++.dg/pr49070.mm   | 52 +++++++++++++++++++++++++++
>> gcc/testsuite/objc.dg/unnamed-parms.m | 28 +++++++++++++++
>> 4 files changed, 91 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/obj-c++.dg/pr49070.mm
>> create mode 100644 gcc/testsuite/objc.dg/unnamed-parms.m
>>
>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>> index 70775792161..e5ba1dcffaa 100644
>> --- a/gcc/cp/parser.c
>> +++ b/gcc/cp/parser.c
>> @@ -572,6 +572,8 @@ cp_debug_parser (FILE *file, cp_parser *parser)
>> 			      parser->colon_corrects_to_scope_p);
>>  cp_debug_print_flag (file, "Colon doesn't start a class definition",
>> 			      parser->colon_doesnt_start_class_def_p);
>> +  cp_debug_print_flag (file, "Parsing an Objective-C++ message context",
>> +			      parser->objective_c_message_context_p);
>>  if (parser->type_definition_forbidden_message)
>>    fprintf (file, "Error message for forbidden type definitions: %s %s\n",
>> 	     parser->type_definition_forbidden_message,
>> @@ -6622,7 +6624,9 @@ cp_parser_nested_name_specifier_opt (cp_parser  
>> *parser,
>>
>> 	  if (token->type == CPP_COLON
>> 	      && parser->colon_corrects_to_scope_p
>> -	      && cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_NAME)
>> +	      && cp_lexer_peek_nth_token (parser->lexer, 3)->type == CPP_NAME
>> +	      /* name:name is a valid sequence in an Objective C message.  */
>> +	      && !parser->objective_c_message_context_p)
>> 	    {
>> 	      gcc_rich_location richloc (token->location);
>> 	      richloc.add_fixit_replace ("::");
>> @@ -32965,6 +32969,7 @@ cp_parser_objc_message_expression (cp_parser*  
>> parser)
>> {
>>  tree receiver, messageargs;
>>
>> +  parser->objective_c_message_context_p = true;
>>  location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
>>  cp_lexer_consume_token (parser->lexer);  /* Eat '['.  */
>>  receiver = cp_parser_objc_message_receiver (parser);
>> @@ -32981,6 +32986,7 @@ cp_parser_objc_message_expression (cp_parser*  
>> parser)
>>  location_t combined_loc = make_location (start_loc, start_loc, end_loc);
>>  protected_set_expr_location (result, combined_loc);
>>
>> +  parser->objective_c_message_context_p = false;
>>  return result;
>> }
>>
>> diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
>> index d587cf2404b..a468b6992ad 100644
>> --- a/gcc/cp/parser.h
>> +++ b/gcc/cp/parser.h
>> @@ -350,6 +350,10 @@ struct GTY(()) cp_parser {
>>     is terminated by colon.  */
>>  bool colon_doesnt_start_class_def_p;
>>
>> +  /* TRUE if we are parsing an objective c message, and ':' is permitted
>> +     to terminate an assignment-expression.  */
>> +  bool objective_c_message_context_p;
>> +
>>  /* If non-NULL, then we are parsing a construct where new type
>>     definitions are not permitted.  The string stored here will be
>>     issued as an error message if a type is defined.  */
>> diff --git a/gcc/testsuite/obj-c++.dg/pr49070.mm  
>> b/gcc/testsuite/obj-c++.dg/pr49070.mm
>> new file mode 100644
>> index 00000000000..d67c72137cc
>> --- /dev/null
>> +++ b/gcc/testsuite/obj-c++.dg/pr49070.mm
>> @@ -0,0 +1,52 @@
>> +/* Only needs to compile.  */
>> +/* { dg-additional-options "-std=c++11" } */
>> +
>> +#ifdef __cplusplus
>> +enum X {
>> +  x = 5,
>> +  y
>> +};
>> +#endif
>> +
>> +__attribute__((__objc_root_class__))
>> +@interface A
>> +- (id) :(id)arg0 :(id)arg1;
>> +- (id) m:(id)arg0 :(id)arg1 :(id)arg2 :(id)arg3;
>> +#ifdef __cplusplus
>> +- (id) n:(X)arg0 :(X)arg1 :(id)arg2 :(id)arg3;
>> +#endif
>> +@end
>> +
>> +@implementation A
>> +- (id) :(id)arg0 :(id)arg1
>> +{
>> +  return arg1;
>> +}
>> +- (id) m:(id)arg0 :(id)arg1 :(id)arg2 :(id)arg3
>> +{
>> +  return arg2;
>> +}
>> +#ifdef __cplusplus
>> +- (id) n:(X)arg0 :(X)arg1 :(id)arg2 :(id)arg3
>> +{
>> +  return arg2;
>> +}
>> +#endif
>> +@end
>> +
>> +id f1 (A *x)
>> +{
>> +  return [x:x:x];
>> +}
>> +
>> +id f2 (A *x)
>> +{
>> +  return [x m:x:x:x:x];
>> +}
>> +
>> +#ifdef __cplusplus
>> +id f3 (A *x)
>> +{
>> +  return [x n:X::x:X::y:x:x];
>> +}
>> +#endif
>> diff --git a/gcc/testsuite/objc.dg/unnamed-parms.m  
>> b/gcc/testsuite/objc.dg/unnamed-parms.m
>> new file mode 100644
>> index 00000000000..22426cfed5f
>> --- /dev/null
>> +++ b/gcc/testsuite/objc.dg/unnamed-parms.m
>> @@ -0,0 +1,28 @@
>> +/* Only needs to compile [see PR 49070 for C++ issue].  */
>> +
>> +__attribute__((__objc_root_class__))
>> +@interface A
>> +- (id) :(id)arg0 :(id)arg1;
>> +- (id) m:(id)arg0 :(id)arg1 :(id)arg2 :(id)arg3;
>> +@end
>> +
>> +@implementation A
>> +- (id) :(id)arg0 :(id)arg1
>> +{
>> +  return arg1;
>> +}
>> +- (id) m:(id)arg0 :(id)arg1 :(id)arg2 :(id)arg3
>> +{
>> +  return arg2;
>> +}
>> +@end
>> +
>> +id f1 (A *x)
>> +{
>> +  return [x:x:x];
>> +}
>> +
>> +id f2 (A *x)
>> +{
>> +  return [x m:x:x:x:x];
>> +}
>> -- 
>> 2.24.1