Skip to content

Commit 38df5e7

Browse files
committed
Preserve symbols interned from C to match MRI semantics.
1 parent 999d5ef commit 38df5e7

File tree

7 files changed

+48
-12
lines changed

7 files changed

+48
-12
lines changed

lib/truffle/truffle/cext.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,10 @@ def rb_thread_alone
723723
Thread.list.count == 1 ? 1 : 0
724724
end
725725

726+
def rb_intern(str)
727+
Primitive.string_to_symbol(str, true)
728+
end
729+
726730
def rb_int_positive_pow(a, b)
727731
a ** b
728732
end

src/main/c/cext/string.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ VALUE rb_str_inspect(VALUE string) {
116116
}
117117

118118
ID rb_intern_str(VALUE string) {
119-
return SYM2ID(RUBY_INVOKE(string, "intern"));
119+
return SYM2ID(RUBY_CEXT_INVOKE("rb_intern", string));
120120
}
121121

122122
VALUE rb_str_cat(VALUE string, const char *to_concat, long length) {

src/main/java/org/truffleruby/RubyLanguage.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,12 @@ public RubySymbol getSymbol(String string) {
383383

384384
@TruffleBoundary
385385
public RubySymbol getSymbol(AbstractTruffleString name, RubyEncoding encoding) {
386-
return symbolTable.getSymbol(name, encoding);
386+
return symbolTable.getSymbol(name, encoding, false);
387+
}
388+
389+
@TruffleBoundary
390+
public RubySymbol getSymbol(AbstractTruffleString name, RubyEncoding encoding, boolean preserveSymbol) {
391+
return symbolTable.getSymbol(name, encoding, preserveSymbol);
387392
}
388393

389394
public Assumption getTracingAssumption() {

src/main/java/org/truffleruby/core/string/StringNodes.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2563,7 +2563,7 @@ public boolean isStringSubclass(RubyString string) {
25632563
}
25642564
}
25652565

2566-
@CoreMethod(names = { "to_sym", "intern" })
2566+
@Primitive(name = "string_to_symbol")
25672567
@ImportStatic(StringGuards.class)
25682568
public abstract static class ToSymNode extends CoreMethodArrayArgumentsNode {
25692569

@@ -2574,27 +2574,28 @@ public abstract static class ToSymNode extends CoreMethodArrayArgumentsNode {
25742574
"!isBrokenCodeRange(tstring, encoding, codeRangeNode)",
25752575
"equalNode.execute(tstring, encoding, cachedTString, cachedEncoding)" },
25762576
limit = "getDefaultCacheLimit()")
2577-
protected RubySymbol toSymCached(Object string,
2577+
protected RubySymbol toSymCached(Object string, boolean preserveSymbol,
25782578
@Cached RubyStringLibrary strings,
25792579
@Cached("asTruffleStringUncached(string)") TruffleString cachedTString,
25802580
@Cached("strings.getEncoding(string)") RubyEncoding cachedEncoding,
2581-
@Cached("getSymbol(cachedTString, cachedEncoding)") RubySymbol cachedSymbol,
2581+
@Cached("preserveSymbol") boolean cachedPreserveSymbol,
2582+
@Cached("getSymbol(cachedTString, cachedEncoding, cachedPreserveSymbol)") RubySymbol cachedSymbol,
25822583
@Cached StringHelperNodes.EqualSameEncodingNode equalNode,
25832584
@Bind("strings.getTString(string)") AbstractTruffleString tstring,
25842585
@Bind("strings.getEncoding(string)") RubyEncoding encoding) {
25852586
return cachedSymbol;
25862587
}
25872588

25882589
@Specialization(guards = "!isBrokenCodeRange(tstring, encoding, codeRangeNode)", replaces = "toSymCached")
2589-
protected RubySymbol toSym(Object string,
2590+
protected RubySymbol toSym(Object string, boolean preserveSymbol,
25902591
@Cached RubyStringLibrary strings,
25912592
@Bind("strings.getTString(string)") AbstractTruffleString tstring,
25922593
@Bind("strings.getEncoding(string)") RubyEncoding encoding) {
2593-
return getSymbol(strings.getTString(string), strings.getEncoding(string));
2594+
return getSymbol(strings.getTString(string), strings.getEncoding(string), preserveSymbol);
25942595
}
25952596

25962597
@Specialization(guards = "isBrokenCodeRange(tstring, encoding, codeRangeNode)")
2597-
protected RubySymbol toSymBroken(Object string,
2598+
protected RubySymbol toSymBroken(Object string, boolean preserveSymbol,
25982599
@Cached RubyStringLibrary strings,
25992600
@Bind("strings.getTString(string)") AbstractTruffleString tstring,
26002601
@Bind("strings.getEncoding(string)") RubyEncoding encoding) {

src/main/java/org/truffleruby/core/symbol/SymbolTable.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.truffleruby.core.symbol;
1111

1212
import java.util.Collection;
13+
import java.util.concurrent.ConcurrentHashMap;
1314

1415
import com.oracle.truffle.api.strings.AbstractTruffleString;
1516
import com.oracle.truffle.api.strings.TruffleString;
@@ -35,6 +36,10 @@ public class SymbolTable {
3536
// As long as the Symbol is referenced, the entry will stay in the symbolMap.
3637
private final WeakValueCache<TStringWithEncoding, RubySymbol> symbolMap = new WeakValueCache<>();
3738

39+
// A map of symbols that should be preserved and never collected,
40+
// like those created by `rb_intern` and similar functions.
41+
private final ConcurrentHashMap<TStringWithEncoding, RubySymbol> preservedSymbolMap = new ConcurrentHashMap<>();
42+
3843
public SymbolTable(TStringCache tstringCache, CoreSymbols coreSymbols) {
3944
this.tstringCache = tstringCache;
4045
addCoreSymbols(coreSymbols);
@@ -75,7 +80,7 @@ public RubySymbol getSymbol(String string) {
7580
str = TStringUtils.utf8TString(string);
7681
encoding = Encodings.UTF_8;
7782
}
78-
symbol = getSymbol(str, encoding);
83+
symbol = getSymbol(str, encoding, false);
7984

8085
// Add it to the direct java.lang.String to Symbol cache
8186
stringToSymbolCache.addInCacheIfAbsent(string, symbol);
@@ -84,19 +89,30 @@ public RubySymbol getSymbol(String string) {
8489
}
8590

8691
@TruffleBoundary
87-
public RubySymbol getSymbol(AbstractTruffleString tstring, RubyEncoding originalEncoding) {
92+
public RubySymbol getSymbol(AbstractTruffleString tstring, RubyEncoding originalEncoding, boolean preserveSymbol) {
8893
var key = normalizeForLookup(tstring, originalEncoding);
89-
final RubySymbol symbol = symbolMap.get(key);
94+
RubySymbol symbol = preservedSymbolMap.get(key);
95+
if (symbol != null) {
96+
return symbol;
97+
}
98+
symbol = symbolMap.get(key);
9099
if (symbol != null) {
100+
if (preserveSymbol) {
101+
preservedSymbolMap.put(key, symbol);
102+
}
91103
return symbol;
92104
}
93105

106+
94107
final RubyEncoding symbolEncoding = key.encoding;
95108
var cachedTString = tstringCache.getTString(key.tstring, symbolEncoding);
96109
final RubySymbol newSymbol = createSymbol(cachedTString, symbolEncoding);
97110
// Use a TStringWithEncoding with the cached TString in symbolMap, since the Symbol refers to it and so we
98111
// do not keep the other TString alive unnecessarily.
99-
return symbolMap.addInCacheIfAbsent(new TStringWithEncoding(cachedTString, symbolEncoding), newSymbol);
112+
var savedSymbol = symbolMap.addInCacheIfAbsent(new TStringWithEncoding(cachedTString, symbolEncoding),
113+
newSymbol);
114+
preservedSymbolMap.put(key, savedSymbol);
115+
return savedSymbol;
100116
}
101117

102118
@TruffleBoundary

src/main/java/org/truffleruby/language/RubyBaseNode.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ protected final RubySymbol getSymbol(AbstractTruffleString name, RubyEncoding en
128128
return getLanguage().getSymbol(name, encoding);
129129
}
130130

131+
protected final RubySymbol getSymbol(AbstractTruffleString name, RubyEncoding encoding, boolean preserveSymbol) {
132+
return getLanguage().getSymbol(name, encoding, preserveSymbol);
133+
}
134+
131135
protected final CoreStrings coreStrings() {
132136
return getLanguage().coreStrings;
133137
}

src/main/ruby/truffleruby/core/string.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,12 @@ def include?(needle)
151151
Primitive.as_boolean(Primitive.find_string(self, StringValue(needle), 0))
152152
end
153153

154+
def intern
155+
Primitive.string_to_symbol(self, false)
156+
end
157+
158+
alias_method :to_sym, :intern
159+
154160
def lstrip
155161
str = Primitive.dup_as_string_instance(self)
156162
str.lstrip! || str

0 commit comments

Comments
 (0)