Re: [PATCH 2/3] Matching tokens: C parts (v2)

[Jeff Law <law@redhat.com>]

On 08/04/2017 08:32 AM, David Malcolm wrote:
> On Thu, 2017-08-03 at 11:34 -0600, Jeff Law wrote:
>> On 08/01/2017 02:21 PM, David Malcolm wrote:
>>> Changed in v2:
>>>
>>> * Renamed template argument to traits_t; eliminated subclasses,
>>> just
>>>   using traits struct.
>>> * Moved enum constants into struct bodies (string constants can't
>>> be
>>>   without constexpr, which isn't available in C++98).
>>> * Fixed typo.
>>>
>>> OK for trunk?
>>>
>>> gcc/c/ChangeLog:
>>> 	* c-parser.c (c_parser_error): Rename to...
>>> 	(c_parser_error_richloc): ...this, making static, and adding
>>> 	"richloc" parameter, passing it to the c_parse_error call,
>>> 	rather than calling c_parser_set_source_position_from_token.
>>> 	(c_parser_error): Reintroduce, reimplementing in terms of the
>>> 	above, converting return type from void to bool.
>>> 	(class token_pair): New class.
>>> 	(struct matching_paren_traits): New struct.
>>> 	(matching_parens): New typedef.
>>> 	(struct matching_brace_traits): New struct.
>>> 	(matching_braces): New typedef.
>>> 	(get_matching_symbol): New function.
>>> 	(c_parser_require): Add param MATCHING_LOCATION, using it to
>>> 	highlight matching "opening" tokens for missing "closing"
>>> tokens.
>>> 	(c_parser_skip_until_found): Likewise.
>>> 	(c_parser_static_assert_declaration_no_semi): Convert explicit
>>> 	parsing of CPP_OPEN_PAREN and CPP_CLOSE_PAREN to use of
>>> 	class matching_parens, so that the pertinent open parenthesis
>>> is
>>> 	highlighted when there are problems locating the close
>>> 	parenthesis.
>>> 	(c_parser_struct_or_union_specifier): Likewise.
>>> 	(c_parser_typeof_specifier): Likewise.
>>> 	(c_parser_alignas_specifier): Likewise.
>>> 	(c_parser_simple_asm_expr): Likewise.
>>> 	(c_parser_braced_init): Likewise, for matching_braces.
>>> 	(c_parser_paren_condition): Likewise, for matching_parens.
>>> 	(c_parser_switch_statement): Likewise.
>>> 	(c_parser_for_statement): Likewise.
>>> 	(c_parser_asm_statement): Likewise.
>>> 	(c_parser_asm_operands): Likewise.
>>> 	(c_parser_cast_expression): Likewise.
>>> 	(c_parser_sizeof_expression): Likewise.
>>> 	(c_parser_alignof_expression): Likewise.
>>> 	(c_parser_generic_selection): Likewise.
>>> 	(c_parser_postfix_expression): Likewise for cases RID_VA_ARG,
>>> 	RID_OFFSETOF, RID_TYPES_COMPATIBLE_P, RID_AT_SELECTOR,
>>> 	RID_AT_PROTOCOL, RID_AT_ENCODE, reindenting as necessary.
>>> 	In case CPP_OPEN_PAREN, pass loc_open_paren to the
>>> 	c_parser_skip_until_found call.
>>> 	(c_parser_objc_class_definition): Use class matching_parens as
>>> 	above.
>>> 	(c_parser_objc_method_decl): Likewise.
>>> 	(c_parser_objc_try_catch_finally_statement): Likewise.
>>> 	(c_parser_objc_synchronized_statement): Likewise.
>>> 	(c_parser_objc_at_property_declaration): Likewise.
>>> 	(c_parser_oacc_wait_list): Likewise.
>>> 	(c_parser_omp_var_list_parens): Likewise.
>>> 	(c_parser_omp_clause_collapse): Likewise.
>>> 	(c_parser_omp_clause_default): Likewise.
>>> 	(c_parser_omp_clause_if): Likewise.
>>> 	(c_parser_omp_clause_num_threads): Likewise.
>>> 	(c_parser_omp_clause_num_tasks): Likewise.
>>> 	(c_parser_omp_clause_grainsize): Likewise.
>>> 	(c_parser_omp_clause_priority): Likewise.
>>> 	(c_parser_omp_clause_hint): Likewise.
>>> 	(c_parser_omp_clause_defaultmap): Likewise.
>>> 	(c_parser_oacc_single_int_clause): Likewise.
>>> 	(c_parser_omp_clause_ordered): Likewise.
>>> 	(c_parser_omp_clause_reduction): Likewise.
>>> 	(c_parser_omp_clause_schedule): Likewise.
>>> 	(c_parser_omp_clause_num_teams): Likewise.
>>> 	(c_parser_omp_clause_thread_limit): Likewise.
>>> 	(c_parser_omp_clause_aligned): Likewise.
>>> 	(c_parser_omp_clause_linear): Likewise.
>>> 	(c_parser_omp_clause_safelen): Likewise.
>>> 	(c_parser_omp_clause_simdlen): Likewise.
>>> 	(c_parser_omp_clause_depend): Likewise.
>>> 	(c_parser_omp_clause_map): Likewise.
>>> 	(c_parser_omp_clause_device): Likewise.
>>> 	(c_parser_omp_clause_dist_schedule): Likewise.
>>> 	(c_parser_omp_clause_proc_bind): Likewise.
>>> 	(c_parser_omp_clause_uniform): Likewise.
>>> 	(c_parser_omp_for_loop): Likewise.
>>> 	(c_parser_cilk_clause_vectorlength): Likewise.
>>> 	(c_parser_cilk_clause_linear): Likewise.
>>> 	(c_parser_transaction_expression): Likewise.
>>> 	* c-parser.h (c_parser_require): Add param matching_location
>>> with
>>> 	default UNKNOWN_LOCATION.
>>> 	(c_parser_error): Convert return type from void to bool.
>>> 	(c_parser_skip_until_found): Add param matching_location with
>>> 	default UNKNOWN_LOCATION.
>>>
>>> gcc/testsuite/ChangeLog:
>>> 	* gcc.dg/unclosed-init.c: New test case.
>>
>> Phew.  I only spot-checked most of the changes around the new API for
>> requiring the open/close paren/brace/bracket or consuming
>> parens/braces/brackets.  They were very mechanical :-)
> 
> Thanks for looking at this.  Do you have an opinion on Trevor's idea
> the the "parser" argument should be moved into the token_pair class (to
> avoid manually passing it in everywhere), or should it be kept outside
> and passed in as needed? 
No opinion.  I'd put it where ever it makes the  most logical sense in
terms of code readability -- until such a point as it's shown to be a
bottleneck.

> 
> I was worried about increasing register pressure in the parsers, since
> it's not clear to me that the optimizer can always prove that a field
> "token_pair::m_parser" in a local token_pair is equal to the parser
> local (see:
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00114.html )
The ability to prove they're the same would depend on aliasing
relationships and whether or not the objects escape as well. Odds are it
won't be able to determine that in general.

jeff