prevent complaining about sub attribute order on a my/our/state

[tonycoz]

fixes #19245

It might be better to clear seen_sig after a sub is parsed, I'm not sure.

[leonerd]

What about attributes on signatured lexical subs? Currently they complain:

use feature 'signatures';
sub MODIFY_CODE_ATTRIBUTES($p, $t, @at) {}
my sub f ($arg) :Final {}

The signatures feature is experimental at - line 2.
The signatures feature is experimental at - line 3.
Subroutine attributes must come before the signature at - line 3.

[leonerd]

I should clarify my comment: The fix skips the warning conditionally on in_my. But my sub ... will also be in_my. So I believe the fix will stop warning about the attribute-vs-signature order of my sub declarations

[iabyn]

Yeah, ideally sig_seen should be turned off in the sigsubbody grammar rule just before the PERLY_BRACE_OPEN. It's only a flag intended to be checked by the lexer at the point X in 'sub foo (...) X { .... }', where X is expected to be empty, but if an attribute is seen by the lexer at this point, give a better error message. (At some point in the future I intend to have a whole series of flags in PL_parser indicating exactly what point in processing signatures is at, but sig_seen is a crude approximation for now.)

Do you want to try this or leave it to me?

[tonycoz]

I'll take a look at it.

[tonycoz]

What about attributes on signatured lexical subs? Currently they complain:

use feature 'signatures';
sub MODIFY_CODE_ATTRIBUTES($p, $t, @at) {}
my sub f ($arg) :Final {}

The signatures feature is experimental at - line 2.
The signatures feature is experimental at - line 3.
Subroutine attributes must come before the signature at - line 3.

This code still warns with my change, I think because PL_in_my is set to 0 once a signature variable is processed in S_pending_ident().

The simpler case:

my sub g () :Final {}

incorrectly doesn't warn, as S_pending_ident() doesn't get a chance to run. I'll try Dave's suggested change.

[tonycoz]

incorrectly doesn't ~~warn~~croak, as S_pending_ident() doesn't get a chance to run. I'll try Dave's suggested change.

It turned out my test was broken.

At the point this error is produced PL_in_my has already been set to 0 in the rule in perly.y:

	|	SIGSUB subname startsub
                    /* sub declaration or definition under 'use feature
                     * "signatures"'. (Note that a signature isn't
                     * allowed in a declaration)
                     */
			{
                          init_named_cv(PL_compcv, $subname);
			  parser->in_my = 0;
			  parser->in_my_stash = NULL;
			}

Adding parser->sig_seen = 0; between the optsubsignature and PERLY_BRACE_OPEN parts of the sigsubbody rule caused test failures, adding it after the PERLY_BRACE_OPEN and removing my fix allowed this to pass.

[tonycoz]

I've switched to using sig_seen here.

The clear needs to happen after the PERLY_BRACE_OPEN otherwise the signature is parsed, sig_seen is cleared, and then the incorrect attributes are parsed with sig_seen == 0, producing the ugly syntax error we're trying to avoid.

I think I was confused (and I'm still somewhat confused) by the SAVEBOOL() in yyl_sub():

    SAVEBOOL(PL_parser->sig_seen);
    PL_parser->sig_seen = FALSE;

but there doesn't appear to be an ENTER/LEAVE pair covering each sub to restore sig_seen here. If I add:

    PerlIO_printf(PerlIO_stderr(), "save stack at %d\n", (int)PL_savestack_ix);

just after this and run:

$ ./miniperl -e 'use v5.34.0; sub foo ($x) { 1 } sub bar ($y) { 1 } sub quux ($z) { 1 }'
save stack at 52
save stack at 54
save stack at 56

the SAVEBOOL just appears to be growing the save stack, though presumably it's more useful for subs defined within these top-level subs.