Hi again,

On 18/05/2018 02:31, Paolo Carlini wrote:

On 18/05/2018 01:21, Jason Merrill wrote:
On Thu, May 17, 2018 at 5:54 PM, Paolo Carlini <paolo.carl...@oracle.com> wrote:
On 17/05/2018 16:58, Jason Merrill wrote:
On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
<paolo.carl...@oracle.com> wrote:
PS: maybe better using function_declarator_p?
I think so, yes.  The relevant rule seems to be "The declarator shall
not specify a function or an array.", so let's check for arrays, too.
Agreed. I had the amended patch ready when I noticed (again) that it wasn't addressing another related class of issues which involves declarators not followed by initializers. Thus I tried to fix those too, and the below which moves the check up appears to work fine, passes testing, etc. Are there any risks that an erroneous function / array as declarator is in fact a well
formed expression?!?
That doesn't matter; if it parses as a declarator, it's a declarator,
even if it's an ill-formed declarator.  But...

+      bool decl_p = cp_parser_parse_definitely (parser);
+      if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+        return error_mark_node;

...if cp_parser_parse_definitely returns false, parsing as a
declarator failed, so we shouldn't look at "declarator".
Uhm, then you are saying that we should fix cp_parser_declarator itself, right? Because we don't want cp_parser_parse_definitely returning false after cp_parser_declarator parses, say, 'if (int foo())' and therefore cp_parser_condition proceed with cp_parser_expression, we want to emit our error and bail out. Therefore the problem in the new patch seems that it tries to paper over that cp_parser_declarator issue in the caller?!? Like, Ok, cp_parser_declarator failed, but it was anyway trying to declare a function / array and that can't possibly be an expression, see what I mean? *Somehow*, the question you answered above.
Ok, now I finally see what's the exact issue you pointed out (I'm a bit tired). Seems fixable.

If I understand correctly, the reason why the 3 lines you cited above are wrong as they are is that my patch *assumes* that cp_parser_declarator didn't really fail and cp_parser_condition has forced the tentative parse to fail by calling cp_parser_simulate_error immediately before when it didn't see an initializer immediately following. That's actually true for 'if (int foo())', thus it makes sense to check the declarator anyway for such cases *even* if cp_parser_parse_definitely returns false. See what I mean?

Therefore, it seems to me that an amended patch would rearrange cp_parser_condition to *not* call cp_parser_simulate_error for the cases we care about ('if (int foo())') and instead check the declarator.

I'll work on that tomorrow...


Reply via email to