detect sub attributes following a signature

RT #132760

A recent commit (v5.27.7-212-g894f226) moved subroutine attributes back
before the subroutine's signature: e.g.

    sub foo :prototype($$) ($a, $b) { ... }  # 5.18 and 5.28 +
    sub foo ($a, $b) :prototype($$) { ... }  # 5.20 .. 5.26

This change means that any code still using an attribute following the
signature is going to trigger a syntax error. However, the error, followed
by error recovery and further warnings and errors, is very unfriendly and
gives no indication of the root cause. This commit introduces a new error,
"Subroutine attributes must come before the signature".

For example, List::Lazy, the subject of the ticket, failed to compile
tests, with output like:

    Array found where operator expected at blib/lib/List/Lazy.pm line 43,
    near "$$@)" (Missing operator before @)?)
    "my" variable $step masks earlier declaration in same statement at
    blib/lib/List/Lazy.pm line 44.
    syntax error at blib/lib/List/Lazy.pm line 36, near ") :"
    Global symbol "$generator" requires explicit package name (did you
    forget to declare "my $generator"?) at blib/lib/List/Lazy.pm line 38.
    Global symbol "$state" requires explicit package name (did you forget
    to declare "my $state"?) at blib/lib/List/Lazy.pm line 39.
    Global symbol "$min" requires explicit package name (did you forget to
    declare "my $min"?) at blib/lib/List/Lazy.pm line 43.
    Global symbol "$max" requires explicit package name (did you forget to
    declare "my $max"?) at blib/lib/List/Lazy.pm line 43.
    Global symbol "$step" requires explicit package name (did you forget
    to declare "my $step"?) at blib/lib/List/Lazy.pm line 43.
    Invalid separator character '{' in attribute list at
    blib/lib/List/Lazy.pm line 44, near "$step : sub "
    Global symbol "$step" requires explicit package name (did you forget
    to declare "my $step"?) at blib/lib/List/Lazy.pm line 44.

But following this commit, it now just outputs:

    Subroutine attributes must come before the signature at
    blib/lib/List/Lazy.pm line 36.
    Compilation failed in require at t/append.t line 5.
    BEGIN failed--compilation aborted at t/append.t line 5.

It works by:

1) adding a boolean flag (sig_seen) to the parser state to indicate that a
   signature has been parsed;
2) at the end of parsing a signature, PL_expect is set to XATTRBLOCK
   rather than XBLOCK.

Then if something looking like one or more attributes is encountered
by the lexer immediately afterwards, it scans it as if it were an
attribute, but then if sig_seen is true, it croaks.

1 files changed, 33 insertions, 10 deletions

diff --git a/toke.c b/toke.c
index df1d7fe15b..9dbad98408 100644
--- a/toke.c
+++ b/toke.c
@@ -5944,9 +5944,17 @@ Perl_yylex(pTHX)
 	case XATTRTERM:
 	    PL_expect = XTERMBLOCK;
 	 grabattrs:
+            /* NB: as well as parsing normal attributes, we also end up
+             * here if there is something looking like attributes
+             * following a signature (which is illegal, but used to be
+             * legal in 5.20..5.26). If the latter, we still parse the
+             * attributes so that error messages(s) are less confusing,
+             * but ignore them (parser->sig_seen).
+             */
 	    s = skipspace(s);
 	    attrs = NULL;
             while (isIDFIRST_lazy_if_safe(s, PL_bufend, UTF)) {
+                bool sig = PL_parser->sig_seen;
 		I32 tmp;
 		SV *sv;
 		d = scan_word(s, PL_tokenbuf, sizeof PL_tokenbuf, FALSE, &len);
@@ -5989,23 +5997,27 @@ Perl_yylex(pTHX)
 		       the CVf_BUILTIN_ATTRS define in cv.h! */
 		    if (!PL_in_my && memEQs(SvPVX(sv), len, "lvalue")) {
 			sv_free(sv);
-			CvLVALUE_on(PL_compcv);
+			if (!sig)
+                            CvLVALUE_on(PL_compcv);
 		    }
 		    else if (!PL_in_my && memEQs(SvPVX(sv), len, "method")) {
 			sv_free(sv);
-			CvMETHOD_on(PL_compcv);
+			if (!sig)
+                            CvMETHOD_on(PL_compcv);
 		    }
 		    else if (!PL_in_my && memEQs(SvPVX(sv), len, "const"))
 		    {
 			sv_free(sv);
-			Perl_ck_warner_d(aTHX_
-			    packWARN(WARN_EXPERIMENTAL__CONST_ATTR),
-			   ":const is experimental"
-			);
-			CvANONCONST_on(PL_compcv);
-			if (!CvANON(PL_compcv))
-			    yyerror(":const is not permitted on named "
-				    "subroutines");
+                        if (!sig) {
+                            Perl_ck_warner_d(aTHX_
+                                packWARN(WARN_EXPERIMENTAL__CONST_ATTR),
+                               ":const is experimental"
+                            );
+                            CvANONCONST_on(PL_compcv);
+                            if (!CvANON(PL_compcv))
+                                yyerror(":const is not permitted on named "
+                                        "subroutines");
+                        }
 		    }
 		    /* After we've set the flags, it could be argued that
 		       we don't need to do the attributes.pm-based setting
@@ -6058,6 +6070,14 @@ Perl_yylex(pTHX)
 		}
 	    }
 	got_attrs:
+            if (PL_parser->sig_seen) {
+                /* see comment about about sig_seen and parser error
+                 * handling */
+                if (attrs)
+                    op_free(attrs);
+                Perl_croak(aTHX_ "Subroutine attributes must come "
+                                 "before the signature");
+                }
 	    if (attrs) {
 		NEXTVAL_NEXTTOKE.opval = attrs;
 		force_next(THING);
@@ -8669,6 +8689,9 @@ Perl_yylex(pTHX)
 		s = skipspace(s);
                 d = SvPVX(PL_linestr)+off;
 
+                SAVEBOOL(PL_parser->sig_seen);
+                PL_parser->sig_seen = FALSE;
+
                 if (   isIDFIRST_lazy_if_safe(s, PL_bufend, UTF)
                     || *s == '\''
                     || (*s == ':' && s[1] == ':'))