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

NFC and NFD objects no longer smartmatch as strings ("7\x[308]".NFD ~~ /^ \d+ $/) #5951

Closed
p6rt opened this issue Dec 30, 2016 · 7 comments
Closed
Labels
regression Issue did not exist previously testneeded

Comments

@p6rt
Copy link

p6rt commented Dec 30, 2016

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

Searchable as RT130458$

@p6rt
Copy link
Author

p6rt commented Dec 30, 2016

From @AlexDaniel

Code​:
say "7\x[308]".NFC ~~ /^ \d+ $/

Result (2015.12,2016.10)​:
「7̈」

Result (2016.11,HEAD)​:
「55」

Previously it worked with NFC objects as if they were strings, which intuitively makes sense. However, I'm not going to claim that this is what it should do, I'm just pointing out that the behavior was changed.

Right now it works with the .ord of the first character, this is probably less than awesome.

The change is a result of both rakudo/rakudo@05b65d0 and rakudo/rakudo@33eeb32

@p6rt
Copy link
Author

p6rt commented Dec 30, 2016

From @lizmat

Fixed with 8d35951 , tests needed

On 30 Dec 2016, at 18​:59, Aleks-Daniel Jakimenko-Aleksejev (via RT) <perl6-bugs-followup@​perl.org> wrote​:

# New Ticket Created by Aleks-Daniel Jakimenko-Aleksejev
# Please include the string​: [perl #​130458]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=130458 >

Code​:
say "7\x[308]".NFC ~~ /^ \d+ $/

Result (2015.12,2016.10)​:
「7̈」

Result (2016.11,HEAD)​:
「55」

Previously it worked with NFC objects as if they were strings, which intuitively makes sense. However, I'm not going to claim that this is what it should do, I'm just pointing out that the behavior was changed.

Right now it works with the .ord of the first character, this is probably less than awesome.

The change is a result of both rakudo/rakudo@05b65d0 and rakudo/rakudo@33eeb32

@p6rt
Copy link
Author

p6rt commented Dec 30, 2016

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

@p6rt
Copy link
Author

p6rt commented Dec 31, 2016

From @usev6

Unfortunately commit 8d35951 breaks the build on JVM. It dies during stage parse with

Error while compiling, type X​::Parameter​::InvalidType
  suggestions​: ()
  typename​: Uni
at line 32953, near " $uni) { "
  in panic (gen/jvm/stage2/NQPHLL.nqp​:348)
  [...]

Type Uni is not usable with rakudo-j​:

$ perl6-j -e 'my Uni $foo'
===SORRY!===
Type 'Uni' is not declared
at -e​:1
------> my Uni⏏ $foo
Malformed my
at -e​:1
------> my⏏ Uni $foo

I'd suggest to add a special case for jvm, so that fix from commit 8d35951 is not applied there. I opened a PR for that​: rakudo/rakudo#980

1 similar comment
@p6rt
Copy link
Author

p6rt commented Dec 31, 2016

From @usev6

Unfortunately commit 8d35951 breaks the build on JVM. It dies during stage parse with

Error while compiling, type X​::Parameter​::InvalidType
  suggestions​: ()
  typename​: Uni
at line 32953, near " $uni) { "
  in panic (gen/jvm/stage2/NQPHLL.nqp​:348)
  [...]

Type Uni is not usable with rakudo-j​:

$ perl6-j -e 'my Uni $foo'
===SORRY!===
Type 'Uni' is not declared
at -e​:1
------> my Uni⏏ $foo
Malformed my
at -e​:1
------> my⏏ Uni $foo

I'd suggest to add a special case for jvm, so that fix from commit 8d35951 is not applied there. I opened a PR for that​: rakudo/rakudo#980

@p6rt
Copy link
Author

p6rt commented Dec 31, 2016

From @moritz

Tested in https://rt-archive.perl.org/perl6//Public/Bug/Display.html?id=130458

Note that this is still not perfect, for example $/.orig doesn't contain an NFD after the match, but that's not in the scope of this ticket.

@p6rt p6rt closed this as completed Dec 31, 2016
@p6rt
Copy link
Author

p6rt commented Dec 31, 2016

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

@p6rt p6rt added regression Issue did not exist previously testneeded labels Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Issue did not exist previously testneeded
Projects
None yet
Development

No branches or pull requests

1 participant