diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ac36fa3ae68..3a181f89ecd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Compatibility: * Allow null encoding pointer in `rb_enc_interned_str_cstr` (@thomasmarshall). * Allow anonymous memberless Struct (@simonlevasseur). * Set `$!` when a `Kernel#at_exit` hook raises an exception (#3535, @andrykonchin). +* Support `:buffer` keyword argument to `Array#pack` (#3559, @andrykonchyn). Performance: * Fix inline caching for Regexp creation from Strings (#3492, @andrykonchin, @eregon). diff --git a/spec/ruby/core/array/pack/buffer_spec.rb b/spec/ruby/core/array/pack/buffer_spec.rb index f1206efb3e84..b77b2d1efa50 100644 --- a/spec/ruby/core/array/pack/buffer_spec.rb +++ b/spec/ruby/core/array/pack/buffer_spec.rb @@ -28,6 +28,16 @@ TypeError, "buffer must be String, not Array") end + it "raise FrozenError if buffer is frozen" do + -> { [65].pack("c", buffer: "frozen-string".freeze) }.should raise_error(FrozenError) + end + + it "preserves the encoding of the given buffer" do + buffer = ''.encode(Encoding::ISO_8859_1) + [65, 66, 67].pack("ccc", buffer: buffer) + buffer.encoding.should == Encoding::ISO_8859_1 + end + context "offset (@) is specified" do it 'keeps buffer content if it is longer than offset' do n = [ 65, 66, 67 ] diff --git a/spec/tags/core/array/pack/buffer_tags.txt b/spec/tags/core/array/pack/buffer_tags.txt deleted file mode 100644 index 299e0d78b44c..000000000000 --- a/spec/tags/core/array/pack/buffer_tags.txt +++ /dev/null @@ -1,6 +0,0 @@ -fails:Array#pack with :buffer option returns specified buffer -fails:Array#pack with :buffer option adds result at the end of buffer content -fails:Array#pack with :buffer option raises TypeError exception if buffer is not String -fails:Array#pack with :buffer option offset (@) is specified keeps buffer content if it is longer than offset -fails:Array#pack with :buffer option offset (@) is specified fills the gap with \0 if buffer content is shorter than offset -fails:Array#pack with :buffer option offset (@) is specified does not keep buffer content if it is longer than offset + result diff --git a/src/main/java/org/truffleruby/core/array/ArrayNodes.java b/src/main/java/org/truffleruby/core/array/ArrayNodes.java index 66590e273968..8bb219de7ae7 100644 --- a/src/main/java/org/truffleruby/core/array/ArrayNodes.java +++ b/src/main/java/org/truffleruby/core/array/ArrayNodes.java @@ -57,6 +57,7 @@ import org.truffleruby.core.encoding.RubyEncoding; import org.truffleruby.core.format.BytesResult; import org.truffleruby.core.format.FormatExceptionTranslator; +import org.truffleruby.core.format.FormatRootNode; import org.truffleruby.core.format.exceptions.FormatException; import org.truffleruby.core.format.pack.PackCompiler; import org.truffleruby.core.hash.HashingNodes; @@ -1491,15 +1492,15 @@ public void accept(Node node, CallBlockNode yieldNode, RubyArray array, Object s } - @CoreMethod(names = "pack", required = 1, split = Split.ALWAYS) - public abstract static class ArrayPackNode extends CoreMethodArrayArgumentsNode { + @Primitive(name = "array_pack", lowerFixnum = 1) + public abstract static class ArrayPackPrimitiveNode extends PrimitiveArrayArgumentsNode { @Specialization - RubyString pack(RubyArray array, Object format, + RubyString pack(RubyArray array, Object format, Object buffer, @Cached ToStrNode toStrNode, @Cached PackNode packNode) { final var formatAsString = toStrNode.execute(this, format); - return packNode.execute(this, array, formatAsString); + return packNode.execute(this, array, formatAsString, buffer); } } @@ -1508,28 +1509,35 @@ RubyString pack(RubyArray array, Object format, @ReportPolymorphism public abstract static class PackNode extends RubyBaseNode { - public abstract RubyString execute(Node node, RubyArray array, Object format); + public abstract RubyString execute(Node node, RubyArray array, Object format, Object buffer); @Specialization( guards = { "libFormat.isRubyString(format)", + "libBuffer.isRubyString(buffer)", "equalNode.execute(libFormat, format, cachedFormat, cachedEncoding)" }, limit = "getCacheLimit()") - static RubyString packCached(Node node, RubyArray array, Object format, + static RubyString packCached(Node node, RubyArray array, Object format, Object buffer, @Cached @Shared InlinedBranchProfile exceptionProfile, @Cached @Shared InlinedConditionProfile resizeProfile, @Cached @Shared RubyStringLibrary libFormat, + @Cached @Shared RubyStringLibrary libBuffer, @Cached @Shared WriteObjectFieldNode writeAssociatedNode, @Cached @Shared TruffleString.FromByteArrayNode fromByteArrayNode, + @Cached @Shared TruffleString.CopyToByteArrayNode copyToByteArrayNode, @Cached("asTruffleStringUncached(format)") TruffleString cachedFormat, @Cached("libFormat.getEncoding(format)") RubyEncoding cachedEncoding, @Cached("cachedFormat.byteLength(cachedEncoding.tencoding)") int cachedFormatLength, - @Cached("create(compileFormat(node, getJavaString(format)))") DirectCallNode callPackNode, + @Cached("compileFormat(node, getJavaString(format))") RootCallTarget formatCallTarget, + @Cached("create(formatCallTarget)") DirectCallNode callPackNode, @Cached StringHelperNodes.EqualNode equalNode) { + final byte[] bytes = initOutputBytes(buffer, libBuffer, formatCallTarget, copyToByteArrayNode); + final BytesResult result; + try { result = (BytesResult) callPackNode.call( - new Object[]{ array.getStore(), array.size, false, null }); + new Object[]{ array.getStore(), array.size, bytes, libBuffer.byteLength(buffer) }); } catch (FormatException e) { exceptionProfile.enter(node); throw FormatExceptionTranslator.translate(getContext(node), node, e); @@ -1538,22 +1546,28 @@ static RubyString packCached(Node node, RubyArray array, Object format, return finishPack(node, cachedFormatLength, result, resizeProfile, writeAssociatedNode, fromByteArrayNode); } - @Specialization(guards = { "libFormat.isRubyString(format)" }, replaces = "packCached") - static RubyString packUncached(Node node, RubyArray array, Object format, + @Specialization(guards = { "libFormat.isRubyString(format)", "libBuffer.isRubyString(buffer)" }, + replaces = "packCached") + static RubyString packUncached(Node node, RubyArray array, Object format, Object buffer, @Cached @Shared InlinedBranchProfile exceptionProfile, @Cached @Shared InlinedConditionProfile resizeProfile, @Cached @Shared RubyStringLibrary libFormat, + @Cached @Shared RubyStringLibrary libBuffer, @Cached @Shared WriteObjectFieldNode writeAssociatedNode, @Cached @Shared TruffleString.FromByteArrayNode fromByteArrayNode, + @Cached @Shared TruffleString.CopyToByteArrayNode copyToByteArrayNode, @Cached ToJavaStringNode toJavaStringNode, @Cached IndirectCallNode callPackNode) { final String formatString = toJavaStringNode.execute(node, format); + final RootCallTarget formatCallTarget = compileFormat(node, formatString); + final byte[] bytes = initOutputBytes(buffer, libBuffer, formatCallTarget, copyToByteArrayNode); final BytesResult result; + try { result = (BytesResult) callPackNode.call( - compileFormat(node, formatString), - new Object[]{ array.getStore(), array.size, false, null }); + formatCallTarget, + new Object[]{ array.getStore(), array.size, bytes, libBuffer.byteLength(buffer) }); } catch (FormatException e) { exceptionProfile.enter(node); throw FormatExceptionTranslator.translate(getContext(node), node, e); @@ -1591,6 +1605,20 @@ protected static RootCallTarget compileFormat(Node node, String format) { } } + private static byte[] initOutputBytes(Object buffer, RubyStringLibrary libBuffer, + RootCallTarget formatCallTarget, TruffleString.CopyToByteArrayNode copyToByteArrayNode) { + int bufferLength = libBuffer.byteLength(buffer); + var formatRootNode = (FormatRootNode) formatCallTarget.getRootNode(); + int expectedLength = formatRootNode.getExpectedLength(); + + // output buffer should be at least expectedLength to not mess up the expectedLength's logic + final int length = Math.max(bufferLength, expectedLength); + final byte[] bytes = new byte[length]; + copyToByteArrayNode.execute(libBuffer.getTString(buffer), 0, bytes, 0, bufferLength, + libBuffer.getTEncoding(buffer)); + return bytes; + } + protected int getCacheLimit() { return getLanguage().options.PACK_CACHE; } diff --git a/src/main/java/org/truffleruby/core/format/FormatRootNode.java b/src/main/java/org/truffleruby/core/format/FormatRootNode.java index bb013b1a3bae..d3a3da5b15b0 100644 --- a/src/main/java/org/truffleruby/core/format/FormatRootNode.java +++ b/src/main/java/org/truffleruby/core/format/FormatRootNode.java @@ -30,17 +30,25 @@ public final class FormatRootNode extends RubyBaseRootNode implements InternalRo @Child private FormatNode child; @CompilationFinal private int expectedLength = 0; + private final boolean acceptOutput; + private final boolean acceptOutputPosition; public FormatRootNode( RubyLanguage language, SourceSection sourceSection, FormatEncoding encoding, - FormatNode child) { + FormatNode child, + boolean acceptOutput, + boolean acceptOutputPosition) { super(language, FormatFrameDescriptor.FRAME_DESCRIPTOR, sourceSection); this.encoding = encoding; this.child = child; + this.acceptOutput = acceptOutput; + this.acceptOutputPosition = acceptOutputPosition; } + /** Accepts the following arguments stored in a frame: source array, its length, output buffer as a bytes array, + * (optional) position in the output buffer to start from */ @SuppressWarnings("unchecked") @Override public Object execute(VirtualFrame frame) { @@ -48,8 +56,13 @@ public Object execute(VirtualFrame frame) { frame.setInt(FormatFrameDescriptor.SOURCE_END_POSITION_SLOT, (int) frame.getArguments()[1]); frame.setInt(FormatFrameDescriptor.SOURCE_START_POSITION_SLOT, 0); frame.setInt(FormatFrameDescriptor.SOURCE_POSITION_SLOT, 0); - frame.setObject(FormatFrameDescriptor.OUTPUT_SLOT, new byte[expectedLength]); - frame.setInt(FormatFrameDescriptor.OUTPUT_POSITION_SLOT, 0); + + final byte[] outputInit = acceptOutput ? (byte[]) frame.getArguments()[2] : new byte[expectedLength]; + frame.setObject(FormatFrameDescriptor.OUTPUT_SLOT, outputInit); + + final int outputPosition = acceptOutputPosition ? (int) frame.getArguments()[3] : 0; + frame.setInt(FormatFrameDescriptor.OUTPUT_POSITION_SLOT, outputPosition); + frame.setObject(FormatFrameDescriptor.ASSOCIATED_SLOT, null); child.execute(frame); @@ -95,6 +108,10 @@ public String getName() { return "format"; } + public int getExpectedLength() { + return expectedLength; + } + @Override public String toString() { return getName(); diff --git a/src/main/java/org/truffleruby/core/format/pack/PackCompiler.java b/src/main/java/org/truffleruby/core/format/pack/PackCompiler.java index 71ea5fceb265..733b6ac1cd89 100644 --- a/src/main/java/org/truffleruby/core/format/pack/PackCompiler.java +++ b/src/main/java/org/truffleruby/core/format/pack/PackCompiler.java @@ -46,7 +46,9 @@ public RootCallTarget compile(String format) throws DeferredRaiseException { language, currentNode.getEncapsulatingSourceSection(), builder.getEncoding(), - builder.getNode()).getCallTarget(); + builder.getNode(), + true, + true).getCallTarget(); } } diff --git a/src/main/java/org/truffleruby/core/format/printf/PrintfCompiler.java b/src/main/java/org/truffleruby/core/format/printf/PrintfCompiler.java index 2f656dd51a21..eb291e44293d 100644 --- a/src/main/java/org/truffleruby/core/format/printf/PrintfCompiler.java +++ b/src/main/java/org/truffleruby/core/format/printf/PrintfCompiler.java @@ -46,7 +46,9 @@ public RootCallTarget compile(AbstractTruffleString tstring, RubyEncoding encodi language, currentNode.getEncapsulatingSourceSection(), new FormatEncoding(encoding), - builder.getNode()).getCallTarget(); + builder.getNode(), + false, + false).getCallTarget(); } } diff --git a/src/main/java/org/truffleruby/core/format/rbsprintf/RBSprintfCompiler.java b/src/main/java/org/truffleruby/core/format/rbsprintf/RBSprintfCompiler.java index 0148c77c474a..41a3a2ea9bed 100644 --- a/src/main/java/org/truffleruby/core/format/rbsprintf/RBSprintfCompiler.java +++ b/src/main/java/org/truffleruby/core/format/rbsprintf/RBSprintfCompiler.java @@ -51,7 +51,9 @@ public RootCallTarget compile(AbstractTruffleString formatTString, RubyEncoding language, currentNode.getEncapsulatingSourceSection(), new FormatEncoding(formatEncoding), - builder.getNode()).getCallTarget(); + builder.getNode(), + false, + false).getCallTarget(); } private static int SIGN = 0x10; diff --git a/src/main/ruby/truffleruby/core/array.rb b/src/main/ruby/truffleruby/core/array.rb index bcf7ecc6b203..3436aed2fb93 100644 --- a/src/main/ruby/truffleruby/core/array.rb +++ b/src/main/ruby/truffleruby/core/array.rb @@ -719,6 +719,20 @@ def last(n = undefined) Array.new self[-n..-1] end + def pack(format, buffer: nil) + if Primitive.nil? buffer + Primitive.array_pack(self, format, '') + else + unless Primitive.is_a?(buffer, String) + raise TypeError, "buffer must be String, not #{Primitive.class(buffer)}" + end + + string = Primitive.array_pack(self, format, buffer) + buffer.replace string.force_encoding(buffer.encoding) + end + end + Truffle::Graal.always_split instance_method(:pack) + def permutation(num = undefined, &block) unless block_given? return to_enum(:permutation, num) do diff --git a/test/truffle/integration/backtraces/pack.backtrace b/test/truffle/integration/backtraces/pack.backtrace index aecf5fddc719..a26f2dab77d4 100644 --- a/test/truffle/integration/backtraces/pack.backtrace +++ b/test/truffle/integration/backtraces/pack.backtrace @@ -1,5 +1,5 @@ /pack.rb:13:in `to_str': message (RuntimeError) - from /pack.rb:18:in `pack' + from core/array.rb:LINE:in `pack' from /pack.rb:18:in `block in
' from /backtraces.rb:17:in `check' from /pack.rb:17:in `
'