Skip to content

Commit d82957a

Browse files
committed
[GR-18163] Fix Kernel#sprintf with %p and nil argument
PullRequest: truffleruby/4543
2 parents f36d95b + 4083291 commit d82957a

File tree

11 files changed

+189
-52
lines changed

11 files changed

+189
-52
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Compatibility:
3131
* Fix `StringIO#{gets,readline}` when it is called with both separator and limit to truncate the separator if the limit is exceeded (#3856, @andrykonchin).
3232
* Implement `rb_error_frozen_object` for the google-protobuf gem (@nirvdrum).
3333
* Adjust a `FrozenError`'s message and add a receiver when a frozen module or class is modified (e.g. by defining or undefining an instance method or by defining a nested module (@andrykonchin).
34+
* Fix `Kernel#sprintf` and `%p` format specification to produce `"nil"` for `nil` argument (#3846, @andrykonchin).
3435

3536
Performance:
3637

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: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2025 Oracle and/or its affiliates. All rights reserved. This
2+
* Copyright (c) 2025 Oracle and/or its affiliates. All rights reserved. This
33
* code is released under a tri EPL/GPL/LGPL license. You can use it,
44
* redistribute it and/or modify it under the terms of the:
55
*
@@ -9,12 +9,10 @@
99
*/
1010
package org.truffleruby.core.format.convert;
1111

12+
import static org.truffleruby.language.dispatch.DispatchConfiguration.PRIVATE_RETURN_MISSING;
13+
1214
import java.nio.charset.StandardCharsets;
1315

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;
1816
import org.truffleruby.core.array.RubyArray;
1917
import org.truffleruby.core.encoding.Encodings;
2018
import org.truffleruby.core.format.FormatNode;
@@ -23,24 +21,24 @@
2321
import org.truffleruby.core.klass.RubyClass;
2422
import org.truffleruby.core.string.RubyString;
2523
import org.truffleruby.core.string.TStringConstants;
26-
import org.truffleruby.language.Nil;
2724
import org.truffleruby.language.dispatch.DispatchNode;
25+
import org.truffleruby.language.library.RubyStringLibrary;
2826

2927
import com.oracle.truffle.api.CompilerDirectives;
3028
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;
3132
import com.oracle.truffle.api.dsl.NodeChild;
3233
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;
34+
import com.oracle.truffle.api.strings.TruffleString;
3635

3736
@NodeChild("value")
3837
public abstract class ToStringNode extends FormatNode {
3938

4039
protected final boolean convertNumbersToStrings;
4140
private final String conversionMethod;
4241
private final boolean inspectOnConversionFailure;
43-
private final Object valueOnNil;
4442
protected final boolean specialClassBehaviour;
4543

4644
@Child private DispatchNode toStrNode;
@@ -50,32 +48,23 @@ public abstract class ToStringNode extends FormatNode {
5048
public ToStringNode(
5149
boolean convertNumbersToStrings,
5250
String conversionMethod,
53-
boolean inspectOnConversionFailure,
54-
Object valueOnNil) {
55-
this(convertNumbersToStrings, conversionMethod, inspectOnConversionFailure, valueOnNil, false);
51+
boolean inspectOnConversionFailure) {
52+
this(convertNumbersToStrings, conversionMethod, inspectOnConversionFailure, false);
5653
}
5754

5855
public ToStringNode(
5956
boolean convertNumbersToStrings,
6057
String conversionMethod,
6158
boolean inspectOnConversionFailure,
62-
Object valueOnNil,
6359
boolean specialClassBehaviour) {
6460
this.convertNumbersToStrings = convertNumbersToStrings;
6561
this.conversionMethod = conversionMethod;
6662
this.inspectOnConversionFailure = inspectOnConversionFailure;
67-
68-
this.valueOnNil = valueOnNil;
6963
this.specialClassBehaviour = specialClassBehaviour;
7064
}
7165

7266
public abstract Object executeToString(Object object);
7367

74-
@Specialization
75-
Object toStringNil(Nil nil) {
76-
return valueOnNil;
77-
}
78-
7968
@Specialization(guards = "convertNumbersToStrings")
8069
RubyString toString(long value,
8170
@Cached TruffleString.FromLongNode fromLongNode) {
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright (c) 2015, 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 org.truffleruby.core.format.FormatNode;
13+
import org.truffleruby.language.Nil;
14+
15+
import com.oracle.truffle.api.CompilerDirectives;
16+
import com.oracle.truffle.api.dsl.NodeChild;
17+
import com.oracle.truffle.api.dsl.Specialization;
18+
19+
@NodeChild("value")
20+
public abstract class ToStringOrDefaultValueNode extends FormatNode {
21+
22+
protected final boolean convertNumbersToStrings;
23+
private final String conversionMethod;
24+
private final boolean inspectOnConversionFailure;
25+
private final Object valueOnNil;
26+
protected final boolean specialClassBehaviour;
27+
28+
@Child private ToStringNode toStringNode;
29+
30+
public ToStringOrDefaultValueNode(
31+
boolean convertNumbersToStrings,
32+
String conversionMethod,
33+
boolean inspectOnConversionFailure,
34+
Object valueOnNil) {
35+
this(convertNumbersToStrings, conversionMethod, inspectOnConversionFailure, valueOnNil, false);
36+
}
37+
38+
public ToStringOrDefaultValueNode(
39+
boolean convertNumbersToStrings,
40+
String conversionMethod,
41+
boolean inspectOnConversionFailure,
42+
Object valueOnNil,
43+
boolean specialClassBehaviour) {
44+
this.convertNumbersToStrings = convertNumbersToStrings;
45+
this.conversionMethod = conversionMethod;
46+
this.inspectOnConversionFailure = inspectOnConversionFailure;
47+
48+
this.valueOnNil = valueOnNil;
49+
this.specialClassBehaviour = specialClassBehaviour;
50+
}
51+
52+
public abstract Object executeToString(Object object);
53+
54+
@Specialization
55+
Object toStringNil(Nil nil) {
56+
return valueOnNil;
57+
}
58+
59+
@Specialization(guards = "!isNil(value)")
60+
Object toString(Object value) {
61+
if (toStringNode == null) {
62+
CompilerDirectives.transferToInterpreterAndInvalidate();
63+
toStringNode = insert(ToStringNodeGen.create(convertNumbersToStrings, conversionMethod,
64+
inspectOnConversionFailure, specialClassBehaviour, null));
65+
}
66+
67+
return toStringNode.executeToString(value);
68+
}
69+
70+
}

src/main/java/org/truffleruby/core/format/pack/SimplePackTreeBuilder.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
import org.truffleruby.core.format.read.SourceNode;
3737
import org.truffleruby.core.format.read.array.ReadDoubleNodeGen;
3838
import org.truffleruby.core.format.read.array.ReadLongOrBigIntegerNodeGen;
39-
import org.truffleruby.core.format.read.array.ReadStringNodeGen;
39+
import org.truffleruby.core.format.read.array.ReadStringOrDefaultValueNodeGen;
4040
import org.truffleruby.core.format.read.array.ReadValueNodeGen;
4141
import org.truffleruby.core.format.write.bytes.WriteBERNodeGen;
4242
import org.truffleruby.core.format.write.bytes.WriteBase64StringNodeGen;
@@ -154,7 +154,7 @@ public void uuString(int count) {
154154
appendNode(WriteUUStringNodeGen.create(
155155
starLength.getLength(),
156156
starLength.isStar(),
157-
ReadStringNodeGen.create(
157+
ReadStringOrDefaultValueNodeGen.create(
158158
false,
159159
"to_str",
160160
false,
@@ -180,7 +180,7 @@ public void mimeString(int count) {
180180

181181
appendNode(WriteMIMEStringNodeGen.create(
182182
length,
183-
ReadStringNodeGen.create(
183+
ReadStringOrDefaultValueNodeGen.create(
184184
true,
185185
"to_s",
186186
true,
@@ -197,7 +197,7 @@ public void base64String(int count) {
197197
appendNode(WriteBase64StringNodeGen.create(
198198
starLength.getLength(),
199199
starLength.isStar(),
200-
ReadStringNodeGen.create(
200+
ReadStringOrDefaultValueNodeGen.create(
201201
false,
202202
"to_str",
203203
false,
@@ -383,7 +383,7 @@ private void binaryString(byte padding, boolean padOnNull, boolean appendNull, i
383383
padding,
384384
takeAll,
385385
appendNull,
386-
ReadStringNodeGen.create(
386+
ReadStringOrDefaultValueNodeGen.create(
387387
false,
388388
"to_str",
389389
false,
@@ -399,7 +399,7 @@ private void bitString(ByteOrder byteOrder, int count) {
399399
byteOrder,
400400
starLength.isStar(),
401401
starLength.getLength(),
402-
ReadStringNodeGen.create(
402+
ReadStringOrDefaultValueNodeGen.create(
403403
false,
404404
"to_str",
405405
false,
@@ -421,7 +421,7 @@ private void hexString(ByteOrder byteOrder, int count) {
421421
appendNode(WriteHexStringNodeGen.create(
422422
byteOrder,
423423
length,
424-
ReadStringNodeGen.create(
424+
ReadStringOrDefaultValueNodeGen.create(
425425
false,
426426
"to_str",
427427
false,

src/main/java/org/truffleruby/core/format/printf/PrintfSimpleTreeBuilder.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@
3838
import org.truffleruby.core.format.read.array.ReadValueNodeGen;
3939
import org.truffleruby.core.format.write.bytes.WriteBytesNodeGen;
4040
import org.truffleruby.core.format.write.bytes.WritePaddedBytesNodeGen;
41-
import org.truffleruby.core.string.FrozenStrings;
42-
import org.truffleruby.core.string.ImmutableRubyString;
4341
import org.truffleruby.core.symbol.RubySymbol;
4442

4543
public final class PrintfSimpleTreeBuilder {
@@ -51,8 +49,6 @@ public final class PrintfSimpleTreeBuilder {
5149

5250
public static final int DEFAULT = Integer.MIN_VALUE;
5351

54-
private static final ImmutableRubyString EMPTY_STRING = FrozenStrings.EMPTY_US_ASCII;
55-
5652
public PrintfSimpleTreeBuilder(RubyLanguage language, List<SprintfConfig> configs, RubyEncoding encoding) {
5753
this.language = language;
5854
this.configs = configs;
@@ -237,11 +233,10 @@ private void buildTree() {
237233
true,
238234
conversionMethodName,
239235
false,
240-
EMPTY_STRING,
241236
new SourceNode());
242237
} else {
243238
conversionNode = ToStringNodeGen
244-
.create(true, conversionMethodName, false, EMPTY_STRING, valueNode);
239+
.create(true, conversionMethodName, false, valueNode);
245240
}
246241

247242
if (config.getWidth() != null || config.isWidthStar() ||

src/main/java/org/truffleruby/core/format/rbsprintf/RBSprintfSimpleTreeBuilder.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@
3838
import org.truffleruby.core.format.read.array.ReadValueNodeGen;
3939
import org.truffleruby.core.format.write.bytes.WriteBytesNodeGen;
4040
import org.truffleruby.core.format.write.bytes.WritePaddedBytesNodeGen;
41-
import org.truffleruby.core.string.FrozenStrings;
42-
import org.truffleruby.core.string.ImmutableRubyString;
4341

4442
public final class RBSprintfSimpleTreeBuilder {
4543

@@ -50,8 +48,6 @@ public final class RBSprintfSimpleTreeBuilder {
5048

5149
public static final int DEFAULT = PrintfSimpleTreeBuilder.DEFAULT;
5250

53-
private static final ImmutableRubyString EMPTY_STRING = FrozenStrings.EMPTY_US_ASCII;
54-
5551
public RBSprintfSimpleTreeBuilder(List<RBSprintfConfig> configs, Object stringReader, RubyEncoding encoding) {
5652
this.configs = configs;
5753
this.stringReader = stringReader;
@@ -283,11 +279,10 @@ private void buildTree() {
283279
true,
284280
conversionMethodName,
285281
false,
286-
EMPTY_STRING,
287282
config.isPlus(),
288283
new SourceNode());
289284
} else {
290-
conversionNode = ToStringNodeGen.create(true, conversionMethodName, false, EMPTY_STRING,
285+
conversionNode = ToStringNodeGen.create(true, conversionMethodName, false,
291286
config.isPlus(), valueNode);
292287
}
293288
} else {
@@ -296,7 +291,6 @@ private void buildTree() {
296291
true,
297292
conversionMethodName,
298293
false,
299-
EMPTY_STRING,
300294
config.isPlus(),
301295
(config.getAbsoluteArgumentIndex() == null)
302296
? (ReadCValueNodeGen

src/main/java/org/truffleruby/core/format/read/array/ReadStringNode.java

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2025 Oracle and/or its affiliates. All rights reserved. This
2+
* Copyright (c) 2025 Oracle and/or its affiliates. All rights reserved. This
33
* code is released under a tri EPL/GPL/LGPL license. You can use it,
44
* redistribute it and/or modify it under the terms of the:
55
*
@@ -12,11 +12,9 @@
1212
import org.truffleruby.core.array.ArrayGuards;
1313
import org.truffleruby.core.array.library.ArrayStoreLibrary;
1414
import org.truffleruby.core.format.FormatNode;
15-
import org.truffleruby.core.format.LiteralFormatNode;
1615
import org.truffleruby.core.format.convert.ToStringNode;
1716
import org.truffleruby.core.format.convert.ToStringNodeGen;
1817
import org.truffleruby.core.format.read.SourceNode;
19-
import org.truffleruby.core.format.write.bytes.WriteByteNodeGen;
2018

2119
import com.oracle.truffle.api.CompilerDirectives;
2220
import com.oracle.truffle.api.dsl.ImportStatic;
@@ -32,29 +30,25 @@ public abstract class ReadStringNode extends FormatNode {
3230
private final boolean convertNumbersToStrings;
3331
private final String conversionMethod;
3432
private final boolean inspectOnConversionFailure;
35-
private final Object valueOnNil;
3633
private final boolean specialClassBehaviour;
3734

3835
@Child private ToStringNode toStringNode;
3936

4037
public ReadStringNode(
4138
boolean convertNumbersToStrings,
4239
String conversionMethod,
43-
boolean inspectOnConversionFailure,
44-
Object valueOnNil) {
45-
this(convertNumbersToStrings, conversionMethod, inspectOnConversionFailure, valueOnNil, false);
40+
boolean inspectOnConversionFailure) {
41+
this(convertNumbersToStrings, conversionMethod, inspectOnConversionFailure, false);
4642
}
4743

4844
public ReadStringNode(
4945
boolean convertNumbersToStrings,
5046
String conversionMethod,
5147
boolean inspectOnConversionFailure,
52-
Object valueOnNil,
5348
boolean specialClassBehaviour) {
5449
this.convertNumbersToStrings = convertNumbersToStrings;
5550
this.conversionMethod = conversionMethod;
5651
this.inspectOnConversionFailure = inspectOnConversionFailure;
57-
this.valueOnNil = valueOnNil;
5852
this.specialClassBehaviour = specialClassBehaviour;
5953
}
6054

@@ -71,9 +65,8 @@ private Object readAndConvert(Object value) {
7165
convertNumbersToStrings,
7266
conversionMethod,
7367
inspectOnConversionFailure,
74-
valueOnNil,
7568
specialClassBehaviour,
76-
WriteByteNodeGen.create(new LiteralFormatNode((byte) 0))));
69+
null));
7770
}
7871

7972
return toStringNode.executeToString(value);

0 commit comments

Comments
 (0)