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

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

Projects

None yet

3 participants

@bjfish
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
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
Member
eregon commented Jan 12, 2017

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

@nirvdrum
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
Member
eregon commented Jan 12, 2017

This caching the mapping of captures names and captured Strings.

@eregon eregon added a commit that referenced this issue Jan 12, 2017
@eregon eregon [Truffle] Do not cache Regexp#named_captures.
* It is not a fast-path operation, neither Regexp#names nor MatchData#names.
* Caching still involves Hash#dup.
* See #4436.
9de5fa0
@eregon eregon closed this Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment