Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backtracking modifiers on individual atoms fail to override a regex-global :ratchet modifier. #6474

Closed
p6rt opened this issue Aug 27, 2017 · 7 comments
Labels
regex Regular expressions, pattern matching, user-defined grammars, tokens and rules testneeded

Comments

@p6rt
Copy link

p6rt commented Aug 27, 2017

Migrated from rt.perl.org#131973 (status was 'resolved')

Searchable as RT131973$

@p6rt
Copy link
Author

p6rt commented Aug 27, 2017

From @smls

Based on S05, these test-cases should all pass​:

  is "ab" ~~ / [ab | a ] b /, "ab", 'normal backtracking';
  is "ab" ~~ / [ab | a ]​: b /, Nil, 'locally disabled backtracking';
  is "ab" ~~ / :r [ab | a ] b /, Nil, 'globally disabled backtracking';
  is "ab" ~~ / :r [ab | a ]​:! b /, "ab", 'globally disabled but
locally re-enabled backtracking';

In current Rakudo, the first three pass but the last one fails (it
refuses to backtrack into the alternation and thus returns no Match).

According to S05 that's a bug​:

"The new :r or :ratchet modifier causes this regex to not backtrack by
default. [...] Explicit backtracking modifiers on quantified atoms[...]
will override this.
-- http://design.perl6.org/S05.html#line_624

Related to RT #​130117.

@p6rt
Copy link
Author

p6rt commented Aug 28, 2017

From @coke

On Sun, 27 Aug 2017 10​:03​:52 -0700, smls75@​gmail.com wrote​:

Based on S05, these test-cases should all pass​:

 is "ab" ~~ / \[ab | a \] b /, "ab", 'normal backtracking';
 is "ab" ~~ / \[ab | a \]​: b /, Nil, 'locally disabled backtracking';
 is "ab" ~~ / :r \[ab | a \] b /, Nil, 'globally disabled backtracking';
 is "ab" ~~ / :r \[ab | a \]​:\! b /, "ab", 'globally disabled but 

locally re-enabled backtracking';

In current Rakudo, the first three pass but the last one fails (it
refuses to backtrack into the alternation and thus returns no Match).

According to S05 that's a bug​:

"The new :r or :ratchet modifier causes this regex to not backtrack by
default. [...] Explicit backtracking modifiers on quantified atoms[...]
will override this.
-- http://design.perl6.org/S05.html#line_624

Related to RT #​130117.

At this stage, something's presence in one of the SYN doesn't mandate that it needs to be present in rakudo.

Tagged ticket as RFC.

--
Will "Coke" Coleda

@p6rt
Copy link
Author

p6rt commented Aug 28, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Aug 28, 2017

From @smls

I sent a pull request to nqp that fixes this, please review​:

Raku/nqp#368

On Mon, 28 Aug 2017 05​:53​:36 -0700, coke wrote​:

At this stage, something's presence in one of the SYN doesn't mandate
that it needs to be present in rakudo.

I'd say this particular issue is pretty clearly a fix for an unintended oversight/bug in Rakudo's implementation of the interaction between two existing features​:

1) `​:r` which controls backtracking for the rest of the lexical scope it's in.
2) `​:` / `​:!` which controls backtracking for a single atom.

The reasonable way for these to interact, is for the the more general one (which also by necessity comes first in the source code) to be overridden by the more specific one (which also comes later in the source code).

The opposite (current Rakudo behavior in the demonstrated edge case) makes no sense, because if combining the general and specific modifier has the same effect as just the general one on its own, then there is not point in combining them in the first place. (And indeed this is also why I don't expect any negative fallout from this change.)

A third option would theoretically be possible​: *Forbid* combining the two features altogether, i.e. throw an exception. But there would have to be a pretty un-obvious reason to choose this over option A, and if there was one, the S05 authors would have probably thought of it.
More importantly, Rakudo *already* does the common-sensical option A for other backtrackable atoms (i.e. quantifiers) - it's just for *alternation* atoms that atom-specific backtracking control is silently ignored when a lexical `​:ratchet` is active.

My patch to fix this¹ also tracked the current behavior to a probable thinko in a nested logic expression.

TL;DR​: This shouldn't be fixed *because* S05 says so, S05 is merely supporting evidence for there not being some obscure reason to not consider this a bug when it looks, quacks, etc. like a bug.
But point taken, I'll learn to de-emphasize² the synopsis viewpoint in future bug reports... :)


1) Raku/nqp@ba165c8
2) I'd also like to point out, though, that not all synopses are created equal - S05 is one of the ones to which Perl 6 has ended up adhering most faithfully (not counting some NYI features). Not consulting it at all, when deciding what is or isn't an unintended bug in Perl 6 regex land, would be a mistake...

@p6rt
Copy link
Author

p6rt commented Oct 4, 2017

From @smls

On Mon, 28 Aug 2017 09​:20​:33 -0700, smls75@​gmail.com wrote​:

I sent a pull request to nqp that fixes this, please review​:

Raku/nqp#368

The PR was merged (and Rakudo's nqp version bumped).

Marking the ticket TESTNEEDED. (Note that some possible tests are listed in the PR description!)

@p6rt p6rt closed this as completed Oct 6, 2017
@p6rt
Copy link
Author

p6rt commented Oct 6, 2017

@smls - Status changed from 'open' to 'resolved'

@p6rt
Copy link
Author

p6rt commented Oct 6, 2017

From @smls

Tests were added here​: Raku/roast@65a762217

@p6rt p6rt added regex Regular expressions, pattern matching, user-defined grammars, tokens and rules testneeded labels Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regex Regular expressions, pattern matching, user-defined grammars, tokens and rules testneeded
Projects
None yet
Development

No branches or pull requests

1 participant