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

Regex Unicode properties check string values before checking bool properties #5966

Closed
p6rt opened this issue Jan 2, 2017 · 7 comments
Closed

Comments

@p6rt
Copy link

p6rt commented Jan 2, 2017

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

Searchable as RT130483$

@p6rt
Copy link
Author

p6rt commented Jan 2, 2017

From @samcv

See test in properties-general.t

The test used to pass before, but only because 'space' resolved to Unicode
Property 'LF'='space'.

Since https://github.com/MoarVM/MoarVM/commit/
5f1e081bad4a4846f2e7a0681af60450e82155c8
or one the commits right before it, this is broken because space no longer
means LF=space.

#?rakudo.moar TODO "Possible bug in NQP where <​:space> does not match,
because it checks property VALUES before checking Bool property names"

@p6rt
Copy link
Author

p6rt commented Jan 2, 2017

From @samcv

say ' ' ~~ /<​:space>/; #> Nil

Should return spaces instead.

Checking this works fine though​:
' '.uniprop-bool('space') #> True

@p6rt
Copy link
Author

p6rt commented Jan 5, 2017

From @samcv

Also see this bisectable results​:
https://gist.github.com/Whateverable/50acf5fe072680085746459f144a106f

You can see how with the new commit, 'space' and 'White_Space' now resolve to the same property. Before 'space' resolved to the LF property, and magically worked. When this was fixed and 'space' == 'White_Space', it broke. This bug will be considered close when​:

use nqp; say nqp​::unipropcode('space') == nqp​::unipropcode('White_Space') #> True
and also must work doing​: `' ' ~~ /<​:space>/; #> 「 」`

@p6rt
Copy link
Author

p6rt commented Mar 16, 2018

From @dogbert17

On Wed, 04 Jan 2017 21​:27​:05 -0800, samantham@​posteo.net wrote​:

Also see this bisectable results​:
https://gist.github.com/Whateverable/50acf5fe072680085746459f144a106f

You can see how with the new commit, 'space' and 'White_Space' now
resolve to the same property. Before 'space' resolved to the LF
property, and magically worked. When this was fixed and 'space' ==
'White_Space', it broke. This bug will be considered close when​:

use nqp; say nqp​::unipropcode('space') ==
nqp​::unipropcode('White_Space') #> True
and also must work doing​: `' ' ~~ /<​:space>/; #> 「 」`

Fixed with commit rakudo/rakudo@49dce16

@p6rt
Copy link
Author

p6rt commented Mar 16, 2018

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

@p6rt
Copy link
Author

p6rt commented Mar 19, 2018

From @samcv

This should be fixed. Though the original bug could be true, it is not visible anymore given the changes where I made sure all property values were distinct for each property name and could not be misaligned in the Unicode database.

Could reopen in the future if there is an issue, but see no reason not to close it.

This current bug is probably not directly related, but I will link it now since it's the closest I have right now to this bug​: MoarVM/MoarVM#818

I haven't fully evaluated the linked bug, but if it turns out to be valid could be useful to have it linked to this one.

@p6rt
Copy link
Author

p6rt commented Mar 19, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant