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.

Sign up for GitHub

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

Jump to bottom

[Truffle] Can't modify frozen Regexp Error for Named Captures #4436

Closed
bjfish opened this issue Jan 12, 2017 · 4 comments
Closed

[Truffle] Can't modify frozen Regexp Error for Named Captures #4436

bjfish opened this issue Jan 12, 2017 · 4 comments
Milestone
Non-Release

Comments

@bjfish
Copy link
Contributor

bjfish commented Jan 12, 2017

Environment

Also found while running in the addressable specs.

Expected Behavior

$ ruby -e 'puts /(?<foo>.)(?<bar>.)/.freeze.named_captures'
{"foo"=>[1], "bar"=>[2]}

Actual Behavior

$ jt p '/(?<foo>.)(?<bar>.)/.freeze.named_captures'
$ /Users/brandonfish/Documents/jruby/bin/jruby -X+T -Xtruffle.core.load_path=/Users/brandonfish/Documents/jruby/truffle/src/main/ruby -Xtruffle.graal.warn_unless=false -e 'p begin /(?<foo>.)(?<bar>.)/.freeze.named_captures end'
/Users/brandonfish/Documents/jruby/truffle/src/main/ruby/core/regexp.rb:312:in `named_captures': can't modify frozen Regexp (RuntimeError)
	from -e:1:in `<main>'

cc'ing: @eregon because it appears he has working on this recently: d88a101

MRI appears to avoid this frozen issue by always re-calculating. However maybe we have a way to cache this while avoiding this frozen error?
@bjfish bjfish added the truffle label Jan 12, 2017
@eregon
Copy link
Member

eregon commented Jan 12, 2017

This is unfortunate. Maybe we can modify the code to avoid writing if frozen?. Or specialize that write to bypass the frozen check.

@eregon
Copy link
Member

eregon commented Jan 12, 2017

Or even simpler, remove the caching since I doubt this is called enough to matter.

@nirvdrum
Copy link
Contributor

I'm not sure which cache you're talking about, but the various regexp caches were added because they made a pretty big difference when testing gems.

@eregon
Copy link
Member

eregon commented Jan 12, 2017

This caching the mapping of captures names and captured Strings.

eregon added a commit that referenced this issue Jan 12, 2017
@eregon
[Truffle] Workaround to avoid errors when trying to cache on a frozen…
6b656fc 
… Regexp.

* Fixes #4436.
eregon added a commit that referenced this issue Jan 12, 2017
@eregon
[Truffle] Do not cache Regexp#named_captures.
9de5fa0 
* It is not a fast-path operation, neither Regexp#names nor MatchData#names.
* Caching still involves Hash#dup.
* See #4436.
@eregon eregon closed this as completed Jan 12, 2017
@enebo enebo modified the milestone: truffle-dev Feb 10, 2017
@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Non-Release
Development

No branches or pull requests
4 participants
@nirvdrum @enebo @bjfish @eregon