Skip to content

Commit ccb4e57

Browse files
andrykonchinandrykonchin
committed
Fix Kernel#sprintf and %p format specification to produce "nil" for nil argument
1 parent 8da615f commit ccb4e57

File tree

9 files changed

+279
-138
lines changed

9 files changed

+279
-138
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Compatibility:
2323
* Fix `Kernel#raise` and don't override `cause` at exception re-raising (#3831, @andrykonchin).
2424
* Return a pointer with `#type_size` of 1 for `Pointer#read_pointer` (@eregon).
2525
* Fix `rb_str_locktmp()` and `rb_str_unlocktmp()` to raise `FrozenError` when string argument is frozen (#3752, @andrykonchin).
26+
* Fix `Kernel#sprintf` and `%p` format specification to produce `"nil"` for `nil` argument (#3846, @andrykonchin).
2627

2728
Performance:
2829

‎spec/ruby/core/kernel/shared/sprintf.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,10 @@ def obj.to_int
362362
obj.should_receive(:inspect).and_return("<inspect-result>")
363363
@method.call("%p", obj).should == "<inspect-result>"
364364
end
365+
366+
it "substitutes 'nil' for nil" do
367+
@method.call("%p", nil).should == "nil"
368+
end
365369
end
366370

367371
describe "s" do

‎spec/ruby/optional/capi/ext/string_spec.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ static VALUE string_spec_rb_str_free(VALUE self, VALUE str) {
440440
static VALUE string_spec_rb_sprintf1(VALUE self, VALUE str, VALUE repl) {
441441
return rb_sprintf(RSTRING_PTR(str), RSTRING_PTR(repl));
442442
}
443+
443444
static VALUE string_spec_rb_sprintf2(VALUE self, VALUE str, VALUE repl1, VALUE repl2) {
444445
return rb_sprintf(RSTRING_PTR(str), RSTRING_PTR(repl1), RSTRING_PTR(repl2));
445446
}

‎spec/ruby/optional/capi/string_spec.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,16 @@ def inspect
10451045
@s.rb_sprintf4(true.class).should == s
10461046
end
10471047

1048+
it "formats nil using to_s if sign not specified in format" do
1049+
s = 'Result: .'
1050+
@s.rb_sprintf3(nil).should == s
1051+
end
1052+
1053+
it "formats nil using inspect if sign specified in format" do
1054+
s = 'Result: nil.'
1055+
@s.rb_sprintf4(nil).should == s
1056+
end
1057+
10481058
it "truncates a string to a supplied precision if that is shorter than the string" do
10491059
s = 'Result: Hel.'
10501060
@s.rb_sprintf5(0, 3, "Hello").should == s

‎src/main/java/org/truffleruby/core/format/convert/ToStringNode.java

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
/*
2+
* Copyright (c) 2025 Oracle and/or its affiliates. All rights reserved. This
3+
* code is released under a tri EPL/GPL/LGPL license. You can use it,
4+
* redistribute it and/or modify it under the terms of the:
5+
*
6+
* Eclipse Public License version 2.0, or
7+
* GNU General Public License version 2, or
8+
* GNU Lesser General Public License version 2.1.
9+
*/
10+
package org.truffleruby.core.format.convert;
11+
12+
import static org.truffleruby.language.dispatch.DispatchConfiguration.PRIVATE_RETURN_MISSING;
13+
14+
import java.nio.charset.StandardCharsets;
15+
16+
import org.truffleruby.core.array.RubyArray;
17+
import org.truffleruby.core.encoding.Encodings;
18+
import org.truffleruby.core.format.FormatNode;
19+
import org.truffleruby.core.format.exceptions.NoImplicitConversionException;
20+
import org.truffleruby.core.kernel.KernelNodes;
21+
import org.truffleruby.core.klass.RubyClass;
22+
import org.truffleruby.core.string.RubyString;
23+
import org.truffleruby.core.string.TStringConstants;
24+
import org.truffleruby.language.dispatch.DispatchNode;
25+
import org.truffleruby.language.library.RubyStringLibrary;
26+
27+
import com.oracle.truffle.api.CompilerDirectives;
28+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
29+
import com.oracle.truffle.api.dsl.Cached;
30+
import com.oracle.truffle.api.dsl.Cached.Exclusive;
31+
import com.oracle.truffle.api.dsl.Cached.Shared;
32+
import com.oracle.truffle.api.dsl.NodeChild;
33+
import com.oracle.truffle.api.dsl.Specialization;
34+
import com.oracle.truffle.api.strings.TruffleString;
35+
36+
@NodeChild("value")
37+
public abstract class ToStringNode extends FormatNode {
38+
39+
protected final boolean convertNumbersToStrings;
40+
private final String conversionMethod;
41+
private final boolean inspectOnConversionFailure;
42+
protected final boolean specialClassBehaviour;
43+
44+
@Child private DispatchNode toStrNode;
45+
@Child private DispatchNode toSNode;
46+
@Child private KernelNodes.ToSNode inspectNode;
47+
48+
public ToStringNode(
49+
boolean convertNumbersToStrings,
50+
String conversionMethod,
51+
boolean inspectOnConversionFailure) {
52+
this(convertNumbersToStrings, conversionMethod, inspectOnConversionFailure, false);
53+
}
54+
55+
public ToStringNode(
56+
boolean convertNumbersToStrings,
57+
String conversionMethod,
58+
boolean inspectOnConversionFailure,
59+
boolean specialClassBehaviour) {
60+
this.convertNumbersToStrings = convertNumbersToStrings;
61+
this.conversionMethod = conversionMethod;
62+
this.inspectOnConversionFailure = inspectOnConversionFailure;
63+
this.specialClassBehaviour = specialClassBehaviour;
64+
}
65+
66+
public abstract Object executeToString(Object object);
67+
68+
@Specialization(guards = "convertNumbersToStrings")
69+
RubyString toString(long value,
70+
@Cached TruffleString.FromLongNode fromLongNode) {
71+
var tstring = fromLongNode.execute(value, Encodings.US_ASCII.tencoding, true);
72+
return createString(tstring, Encodings.US_ASCII);
73+
}
74+
75+
@TruffleBoundary
76+
@Specialization(guards = "convertNumbersToStrings")
77+
RubyString toString(double value,
78+
@Cached TruffleString.FromJavaStringNode fromJavaStringNode) {
79+
return createString(fromJavaStringNode, Double.toString(value), Encodings.US_ASCII);
80+
}
81+
82+
@TruffleBoundary
83+
@Specialization(guards = "specialClassBehaviour")
84+
Object toStringSpecialClass(RubyClass rubyClass,
85+
@Cached @Shared RubyStringLibrary libString) {
86+
if (rubyClass == getContext().getCoreLibrary().trueClass) {
87+
return createString(TStringConstants.TRUE, Encodings.US_ASCII);
88+
} else if (rubyClass == getContext().getCoreLibrary().falseClass) {
89+
return createString(TStringConstants.FALSE, Encodings.US_ASCII);
90+
} else if (rubyClass == getContext().getCoreLibrary().nilClass) {
91+
return createString(TStringConstants.NIL, Encodings.US_ASCII);
92+
} else {
93+
return toString(rubyClass, libString);
94+
}
95+
}
96+
97+
@Specialization(guards = "argLibString.isRubyString(this, string)", limit = "1")
98+
Object toStringString(Object string,
99+
@Cached @Shared RubyStringLibrary libString,
100+
@Cached @Exclusive RubyStringLibrary argLibString) {
101+
if ("inspect".equals(conversionMethod)) {
102+
final Object value = getToStrNode().call(PRIVATE_RETURN_MISSING, string, conversionMethod);
103+
104+
if (libString.isRubyString(this, value)) {
105+
return value;
106+
} else {
107+
throw new NoImplicitConversionException(string, "String");
108+
}
109+
}
110+
return string;
111+
}
112+
113+
@Specialization
114+
Object toString(RubyArray array,
115+
@Cached @Shared RubyStringLibrary libString) {
116+
if (toSNode == null) {
117+
CompilerDirectives.transferToInterpreterAndInvalidate();
118+
toSNode = insert(DispatchNode.create());
119+
}
120+
121+
final Object value = toSNode.call(PRIVATE_RETURN_MISSING, array, "to_s");
122+
123+
if (libString.isRubyString(this, value)) {
124+
return value;
125+
} else {
126+
throw new NoImplicitConversionException(array, "String");
127+
}
128+
}
129+
130+
@Specialization(
131+
guards = { "isNotRubyString(object)", "!isRubyArray(object)", "!isForeignObject(object)" })
132+
Object toString(Object object,
133+
@Cached @Shared RubyStringLibrary libString) {
134+
final Object value = getToStrNode().call(PRIVATE_RETURN_MISSING, object, conversionMethod);
135+
136+
if (libString.isRubyString(this, value)) {
137+
return value;
138+
}
139+
140+
if (inspectOnConversionFailure) {
141+
if (inspectNode == null) {
142+
CompilerDirectives.transferToInterpreterAndInvalidate();
143+
inspectNode = insert(KernelNodes.ToSNode.create());
144+
}
145+
146+
return inspectNode.execute(object);
147+
} else {
148+
throw new NoImplicitConversionException(object, "String");
149+
}
150+
}
151+
152+
@TruffleBoundary
153+
@Specialization(guards = "isForeignObject(object)")
154+
RubyString toStringForeign(Object object,
155+
@Cached TruffleString.FromByteArrayNode fromByteArrayNode) {
156+
return createString(fromByteArrayNode,
157+
object.toString().getBytes(StandardCharsets.UTF_8),
158+
Encodings.UTF_8);
159+
}
160+
161+
private DispatchNode getToStrNode() {
162+
if (toStrNode == null) {
163+
CompilerDirectives.transferToInterpreterAndInvalidate();
164+
toStrNode = insert(DispatchNode.create());
165+
}
166+
return toStrNode;
167+
}
168+
169+
}

‎src/main/java/org/truffleruby/core/format/convert/ToStringOrDefaultValueNode.java

Lines changed: 7 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,12 @@
99
*/
1010
package org.truffleruby.core.format.convert;
1111

12-
import java.nio.charset.StandardCharsets;
13-
14-
import com.oracle.truffle.api.dsl.Cached;
15-
import com.oracle.truffle.api.dsl.Cached.Shared;
16-
import com.oracle.truffle.api.dsl.Cached.Exclusive;
17-
import com.oracle.truffle.api.strings.TruffleString;
18-
import org.truffleruby.core.array.RubyArray;
19-
import org.truffleruby.core.encoding.Encodings;
2012
import org.truffleruby.core.format.FormatNode;
21-
import org.truffleruby.core.format.exceptions.NoImplicitConversionException;
22-
import org.truffleruby.core.kernel.KernelNodes;
23-
import org.truffleruby.core.klass.RubyClass;
24-
import org.truffleruby.core.string.RubyString;
25-
import org.truffleruby.core.string.TStringConstants;
2613
import org.truffleruby.language.Nil;
27-
import org.truffleruby.language.dispatch.DispatchNode;
2814

2915
import com.oracle.truffle.api.CompilerDirectives;
30-
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
3116
import com.oracle.truffle.api.dsl.NodeChild;
3217
import com.oracle.truffle.api.dsl.Specialization;
33-
import org.truffleruby.language.library.RubyStringLibrary;
34-
35-
import static org.truffleruby.language.dispatch.DispatchConfiguration.PRIVATE_RETURN_MISSING;
3618

3719
@NodeChild("value")
3820
public abstract class ToStringOrDefaultValueNode extends FormatNode {
@@ -43,9 +25,7 @@ public abstract class ToStringOrDefaultValueNode extends FormatNode {
4325
private final Object valueOnNil;
4426
protected final boolean specialClassBehaviour;
4527

46-
@Child private DispatchNode toStrNode;
47-
@Child private DispatchNode toSNode;
48-
@Child private KernelNodes.ToSNode inspectNode;
28+
@Child private ToStringNode toStringNode;
4929

5030
public ToStringOrDefaultValueNode(
5131
boolean convertNumbersToStrings,
@@ -76,105 +56,15 @@ Object toStringNil(Nil nil) {
7656
return valueOnNil;
7757
}
7858

79-
@Specialization(guards = "convertNumbersToStrings")
80-
RubyString toString(long value,
81-
@Cached TruffleString.FromLongNode fromLongNode) {
82-
var tstring = fromLongNode.execute(value, Encodings.US_ASCII.tencoding, true);
83-
return createString(tstring, Encodings.US_ASCII);
84-
}
85-
86-
@TruffleBoundary
87-
@Specialization(guards = "convertNumbersToStrings")
88-
RubyString toString(double value,
89-
@Cached TruffleString.FromJavaStringNode fromJavaStringNode) {
90-
return createString(fromJavaStringNode, Double.toString(value), Encodings.US_ASCII);
91-
}
92-
93-
@TruffleBoundary
94-
@Specialization(guards = "specialClassBehaviour")
95-
Object toStringSpecialClass(RubyClass rubyClass,
96-
@Cached @Shared RubyStringLibrary libString) {
97-
if (rubyClass == getContext().getCoreLibrary().trueClass) {
98-
return createString(TStringConstants.TRUE, Encodings.US_ASCII);
99-
} else if (rubyClass == getContext().getCoreLibrary().falseClass) {
100-
return createString(TStringConstants.FALSE, Encodings.US_ASCII);
101-
} else if (rubyClass == getContext().getCoreLibrary().nilClass) {
102-
return createString(TStringConstants.NIL, Encodings.US_ASCII);
103-
} else {
104-
return toString(rubyClass, libString);
105-
}
106-
}
107-
108-
@Specialization(guards = "argLibString.isRubyString(this, string)", limit = "1")
109-
Object toStringString(Object string,
110-
@Cached @Shared RubyStringLibrary libString,
111-
@Cached @Exclusive RubyStringLibrary argLibString) {
112-
if ("inspect".equals(conversionMethod)) {
113-
final Object value = getToStrNode().call(PRIVATE_RETURN_MISSING, string, conversionMethod);
114-
115-
if (libString.isRubyString(this, value)) {
116-
return value;
117-
} else {
118-
throw new NoImplicitConversionException(string, "String");
119-
}
120-
}
121-
return string;
122-
}
123-
124-
@Specialization
125-
Object toString(RubyArray array,
126-
@Cached @Shared RubyStringLibrary libString) {
127-
if (toSNode == null) {
59+
@Specialization(guards = "!isNil(value)")
60+
Object toString(Object value) {
61+
if (toStringNode == null) {
12862
CompilerDirectives.transferToInterpreterAndInvalidate();
129-
toSNode = insert(DispatchNode.create());
63+
toStringNode = insert(ToStringNodeGen.create(convertNumbersToStrings, conversionMethod,
64+
inspectOnConversionFailure, specialClassBehaviour, null));
13065
}
13166

132-
final Object value = toSNode.call(PRIVATE_RETURN_MISSING, array, "to_s");
133-
134-
if (libString.isRubyString(this, value)) {
135-
return value;
136-
} else {
137-
throw new NoImplicitConversionException(array, "String");
138-
}
139-
}
140-
141-
@Specialization(
142-
guards = { "isNotRubyString(object)", "!isRubyArray(object)", "!isForeignObject(object)" })
143-
Object toString(Object object,
144-
@Cached @Shared RubyStringLibrary libString) {
145-
final Object value = getToStrNode().call(PRIVATE_RETURN_MISSING, object, conversionMethod);
146-
147-
if (libString.isRubyString(this, value)) {
148-
return value;
149-
}
150-
151-
if (inspectOnConversionFailure) {
152-
if (inspectNode == null) {
153-
CompilerDirectives.transferToInterpreterAndInvalidate();
154-
inspectNode = insert(KernelNodes.ToSNode.create());
155-
}
156-
157-
return inspectNode.execute(object);
158-
} else {
159-
throw new NoImplicitConversionException(object, "String");
160-
}
161-
}
162-
163-
@TruffleBoundary
164-
@Specialization(guards = "isForeignObject(object)")
165-
RubyString toStringForeign(Object object,
166-
@Cached TruffleString.FromByteArrayNode fromByteArrayNode) {
167-
return createString(fromByteArrayNode,
168-
object.toString().getBytes(StandardCharsets.UTF_8),
169-
Encodings.UTF_8);
170-
}
171-
172-
private DispatchNode getToStrNode() {
173-
if (toStrNode == null) {
174-
CompilerDirectives.transferToInterpreterAndInvalidate();
175-
toStrNode = insert(DispatchNode.create());
176-
}
177-
return toStrNode;
67+
return toStringNode.executeToString(value);
17868
}
17969

18070
}

0 commit comments

Comments
 (0)