Skip to content

Commit

Permalink
Showing 2 changed files with 26 additions and 7 deletions.
10 changes: 5 additions & 5 deletions test/truffle/compiler/pe/language/defined_pe.rb
Original file line number Diff line number Diff line change
@@ -6,8 +6,8 @@
# GNU General Public License version 2
# GNU Lesser General Public License version 2.1

tagged_example "defined?(true) == 'true'", true
tagged_example "defined?(false) == 'false'", true
tagged_example "defined?(self) == 'self'", true
tagged_example "defined?(14) == 'expression'", true
tagged_example "defined?(14 + 2) == 'expression'", true
example "defined?(true) == 'true'", true
example "defined?(false) == 'false'", true
example "defined?(self) == 'self'", true
example "defined?(14) == 'expression'", true
tagged_example "defined?(14 + 2) == 'method'", true
23 changes: 21 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/core/rope/RopeTable.java
Original file line number Diff line number Diff line change
@@ -44,6 +44,13 @@ public Rope getRope(byte[] bytes, Encoding encoding, CodeRange codeRange) {
lock.readLock().unlock();
}

// The only time we should have a null encoding is if we want to find a rope with the same logical byte[] as
// the one supplied to this method. If we've made it this far, no such rope exists, so return null immediately
// to back out of the recursive call.
if (encoding == null) {
return null;
}

lock.writeLock().lock();

try {
@@ -57,7 +64,19 @@ public Rope getRope(byte[] bytes, Encoding encoding, CodeRange codeRange) {
}
}

final Rope rope = RopeOperations.create(bytes, encoding, codeRange);
// At this point, we were unable to find a rope with the same bytes and encoding (i.e., a direct match).
// However, there may still be a rope with the same byte[] and sharing a direct byte[] can still allow some
// reference equality optimizations. So, do another search but with a marker encoding. The only guarantee
// we can make about the resulting rope is that it would have the same logical byte[], but that's good enough
// for our purposes.
final Rope ropeWithSameBytesButDifferentEncoding = getRope(bytes, null ,codeRange);

final Rope rope;
if (ropeWithSameBytesButDifferentEncoding != null) {
rope = RopeOperations.create(ropeWithSameBytesButDifferentEncoding.getBytes(), encoding, codeRange);
} else {
rope = RopeOperations.create(bytes, encoding, codeRange);
}

ropesTable.put(key, new WeakReference<>(rope));

@@ -89,7 +108,7 @@ public boolean equals(Object o) {
if (o instanceof Key) {
final Key other = (Key) o;

return encoding == other.encoding && Arrays.equals(bytes, other.bytes);
return ((encoding == other.encoding) || (encoding == null)) && Arrays.equals(bytes, other.bytes);
}

return false;

1 comment on commit 54efdcd

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chrisseaton This was the other half to making those defined? PE tests pass. It's not a pretty solution, but RopeTable should now share byte arrays for pre-existing ropes with different encodings.

Please sign in to comment.