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

Bug in Hash literals with invalid symbol keys #2848

Closed
nirvdrum opened this issue Feb 2, 2023 · 1 comment
Closed

Bug in Hash literals with invalid symbol keys #2848

nirvdrum opened this issue Feb 2, 2023 · 1 comment

Comments

@nirvdrum
Copy link
Collaborator

nirvdrum commented Feb 2, 2023

While looking at some IRB test failures with @st0012, we came across a case where MRI raises an EncodingError but TruffleRuby raises nothing. Looking quickly at the specs, I didn't see anything in the language specs to test this behavior. The code in question is a Hash literal with a symbol key with an invalid encoding.

puts({"\xAE": 1}.keys)

Note, I didn't print the Hash directly because the invalid key will raise a separate exception on TruffleRuby when calling Hash#inspect.

On MRI:

> ruby -v bad_irb.rb
ruby 3.2.0 (2022-12-25 revision a528908271) [arm64-darwin21.5.0]
bad_irb.rb: invalid symbol in encoding UTF-8 :"\xAE" (EncodingError)

On TruffleRuby:

> jt ruby -v bad_irb.rb
truffleruby 23.0.0-dev-8bb04d8c, like ruby 3.1.3, GraalVM CE JVM [aarch64-darwin]
�
@nirvdrum nirvdrum changed the title Bug in hash literals with invalid symbol keys Bug in Hash literals with invalid symbol keys Feb 3, 2023
st0012 added a commit to ruby/irb that referenced this issue Feb 3, 2023
The test input IRB currently uses happen to hit a compatibility bug in
TruffleRuby, which has been documented in
oracle/truffleruby#2848

Although it'll eventually be fixed, we can make the test case support TruffleRuby
now by tweaking it just a little bit.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
st0012 added a commit to ruby/irb that referenced this issue Feb 3, 2023
The test input IRB currently uses happen to hit a compatibility bug in
TruffleRuby, which has been documented in
oracle/truffleruby#2848

Although it'll eventually be fixed, we can make the test case support TruffleRuby
now by tweaking it just a little bit.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
@andrykonchin
Copy link
Member

Fixed in 145d23e

@andrykonchin andrykonchin added this to the 23.0.0 Release milestone Feb 8, 2023
st0012 added a commit to ruby/irb that referenced this issue Feb 9, 2023
The test input IRB currently uses happen to hit a compatibility bug in
TruffleRuby, which has been documented in
oracle/truffleruby#2848

Although it'll eventually be fixed, we can make the test case support TruffleRuby
now by tweaking it just a little bit.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
st0012 added a commit to ruby/irb that referenced this issue Feb 9, 2023
The test input IRB currently uses happen to hit a compatibility bug in
TruffleRuby, which has been documented in
oracle/truffleruby#2848

Although it'll eventually be fixed, we can make the test case support TruffleRuby
now by tweaking it just a little bit.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
st0012 added a commit to ruby/irb that referenced this issue Feb 9, 2023
* Improve encoding error test case

The test input IRB currently uses happen to hit a compatibility bug in
TruffleRuby, which has been documented in
oracle/truffleruby#2848

Although it'll eventually be fixed, we can make the test case support TruffleRuby
now by tweaking it just a little bit.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>

* Remove redundant TruffleRuby omits/pends

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>

* Use a different way to test warning emission

The test case was added in d08ef68
to verify that IRB emits Ruby warning as expected.

But the subject it uses relies on CRuby's regexp engine, which isn't always
used in other language implementations, like TruffleRuby. That's why we
ended up skipping TruffleRuby in this test case.

Since the test isn't about regexp itself, we can change the testing subject
and just remove the special condition for TruffleRuby.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>

---------

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
matzbot pushed a commit to ruby/ruby that referenced this issue Feb 9, 2023
(ruby/irb#514)

* Improve encoding error test case

The test input IRB currently uses happen to hit a compatibility bug in
TruffleRuby, which has been documented in
oracle/truffleruby#2848

Although it'll eventually be fixed, we can make the test case support TruffleRuby
now by tweaking it just a little bit.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>

* Remove redundant TruffleRuby omits/pends

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>

* Use a different way to test warning emission

The test case was added in ruby/irb@d08ef68
to verify that IRB emits Ruby warning as expected.

But the subject it uses relies on CRuby's regexp engine, which isn't always
used in other language implementations, like TruffleRuby. That's why we
ended up skipping TruffleRuby in this test case.

Since the test isn't about regexp itself, we can change the testing subject
and just remove the special condition for TruffleRuby.

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>

---------

ruby/irb@6fdf4f3e97

Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
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

2 participants