Skip to content

Commit

Permalink
[Truffle] Tag failing encoding specs.
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisseaton committed Jan 2, 2017
1 parent a98a54d commit eeb4272
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions test/truffle/compiler/pe/core/encoding_pe.rb
Expand Up @@ -12,7 +12,7 @@
example "Encoding::ISO_2022_JP.dummy?", true
example "Encoding::UTF_8.dummy?", false

example "Encoding.compatible?('abc', 'def')", Encoding::UTF_8
example "Encoding.compatible?(Encoding::UTF_8, Encoding::US_ASCII)", Encoding::UTF_8
example "Encoding.compatible?(Encoding::UTF_8, Encoding::ASCII_8BIT)", nil
example "Encoding.compatible?('abc', Encoding::US_ASCII)", Encoding::UTF_8
tagged example "Encoding.compatible?('abc', 'def')", Encoding::UTF_8
tagged example "Encoding.compatible?(Encoding::UTF_8, Encoding::US_ASCII)", Encoding::UTF_8
tagged example "Encoding.compatible?(Encoding::UTF_8, Encoding::ASCII_8BIT)", nil
tagged example "Encoding.compatible?('abc', Encoding::US_ASCII)", Encoding::UTF_8

5 comments on commit eeb4272

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nirvdrum This PE tests are failing. I can't understand how they ever passed - getRubyEncoding is a boundary call. Can you make any sense of this?

@eregon
Copy link
Member

@eregon eregon commented on eeb4272 Jan 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5e9106f sounds like the cause. Maybe we should have a GetRubyEncodingNode that caches based on the Encoding instance?
getRubyEncoding is a boundary because of the index check.

@nirvdrum
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Two things happened here:

  1. I split our encoding compatibility check to cache on Encoding instead of the Ruby encoding (basically, what Benoit is suggesting above I think -- it's called NegotiateCompatibleEncodingNode), but in the process I lost the caching on the Ruby encoding object. I can fix that easily enough.

  2. The boundary on getRubyEncoding in fairly recent and I think should probably go away. It used to be the encoding list was a fixed array, but that turned into an ArrayList when Encoding#replicate was implemented.

@nirvdrum
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to follow up, I now see what Benoit was suggesting is something different (and simpler). I'll look at doing that.

@nirvdrum
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aca2d0e.

Please sign in to comment.