Skip to content

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Sep 14, 2020

fixes #18123

@hvds
Copy link
Contributor

hvds commented Sep 14, 2020

I note that pod/perlre.pod still states:

            The use of "\K" inside of another lookaround assertion is allowed,
            but the behaviour is currently not well defined.

@xsawyerx
Copy link
Member

xsawyerx commented Oct 4, 2020

I note that pod/perlre.pod still states:

            The use of "\K" inside of another lookaround assertion is allowed,
            but the behaviour is currently not well defined.

Any suggestion for how to rephrase this entry? Do we want to keep this "not well defined"?

@xsawyerx
Copy link
Member

xsawyerx commented Oct 4, 2020

I could not replicate the podcheck.t test failure. @hvds, any feedback on this before merging?

@hvds
Copy link
Contributor

hvds commented Oct 4, 2020

@hvds, any feedback on this before merging?

I think the pod should explicitly say "The use of "\K" inside of another lookaround assertion is currently not allowed." Optionally it could also add a reference to #18134.

I haven't traced the code to verify if this actually correctly limits the scope of the disallowance, but the tests look like a reasonable selection. I'd be tempted to add some more though:

  qr{(?=(?=x)x\K)x}
  qr{(?=(?!y)x\K)x}
  qr{(?=(?!y\K)x)x}
  qr{(?!(?=x\K)y)x}

... and maybe a couple similarly for lookbehind and for lookahead/behind mixes.

in some cases we want to group a test that shouldn't croak with the
croak tests, in particular to catch regressions when we've
introduced a croak.
this also simplifies the flagging for these assertions, since this
error is now the only thing using in_lookhead and in_lookbehind they
can be combined into a single in_lookaround.

Rather than conditional increment/decrement as we recurse into S_reg
I simply save the value of in_lookaround and restore it before
returning.  Some unsuccessful or restart paths don't do the restore,
but they either result in a croak(), or a restart which reinitialises
in_lookaround anyway.

Also added tests to ensure that all the different zero-width assertions
with content trigger the error.
It was disallowed because:

- it was breaking things (GH Perl#14638)

- at the time, no-one had the tuits and knowledge to select non-buggy
  semantics and implement them.
@tonycoz tonycoz force-pushed the 18123-K-lookwherever branch from d28de60 to c4d2975 Compare October 19, 2020 04:58
@tonycoz tonycoz merged commit 55afc78 into Perl:blead Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect identification of \K (not actually) inside lookahead

3 participants