Skip to content

Commit

Permalink
Showing 4 changed files with 19 additions and 8 deletions.
1 change: 0 additions & 1 deletion spec/truffle/tags/core/string/modulo_tags.txt
Original file line number Diff line number Diff line change
@@ -10,5 +10,4 @@ fails:String#% behaves as if calling Kernel#Float for %g arguments, when the pas
fails:String#% behaves as if calling Kernel#Float for %g arguments, when the passed argument is hexadecimal string
fails:String#% behaves as if calling Kernel#Float for %G arguments, when the passed argument does not respond to #to_ary
fails:String#% behaves as if calling Kernel#Float for %G arguments, when the passed argument is hexadecimal string
fails:String#% when format string contains %<> formats should raise ArgumentError if no hash given
fails:String#% pads with spaces for %E with Inf, -Inf, and NaN
Original file line number Diff line number Diff line change
@@ -28,8 +28,8 @@ public PrintfCompiler(RubyContext context, RubyNode currentNode) {
this.currentNode = currentNode;
}

public CallTarget compile(byte[] format) {
final PrintfSimpleParser parser = new PrintfSimpleParser(bytesToChars(format));
public CallTarget compile(byte[] format, Object[] arguments) {
final PrintfSimpleParser parser = new PrintfSimpleParser(bytesToChars(format), arguments);
final List<SprintfConfig> configs = parser.parse();
final PrintfSimpleTreeBuilder builder = new PrintfSimpleTreeBuilder(context, configs);

Original file line number Diff line number Diff line change
@@ -10,16 +10,19 @@
package org.jruby.truffle.core.format.printf;

import org.jruby.truffle.core.format.exceptions.InvalidFormatException;
import org.jruby.truffle.language.RubyGuards;

import java.util.ArrayList;
import java.util.List;

public class PrintfSimpleParser {

private final char[] source;
private final Object[] arguments;

public PrintfSimpleParser(char[] source) {
public PrintfSimpleParser(char[] source, Object[] arguments) {
this.source = source;
this.arguments = arguments;
}

@SuppressWarnings("fallthrough")
@@ -28,6 +31,7 @@ public List<SprintfConfig> parse() {
ArgType argType = ArgType.NONE;

final int end = source.length;
int argumentIndex = 0;

for (int i = 0; i < end; ) {

@@ -128,6 +132,7 @@ public List<SprintfConfig> parse() {
config.setNamesBytes(charsToBytes(nameBytes));
i = j + 1;
checkNameArg(argType, nameBytes);
checkHash(arguments, argumentIndex);
argType = ArgType.NAMED;
argTypeSet = true;
if (term == '}') {
@@ -250,10 +255,17 @@ public List<SprintfConfig> parse() {
throw new InvalidFormatException("malformed format string - %" + p);
}
}
argumentIndex += 1;
}
return configs;
}

private static void checkHash(Object[] arguments, int argumentIndex) {
if(argumentIndex >= arguments.length ||
!RubyGuards.isRubyHash(arguments[argumentIndex])) {
throw new InvalidFormatException("one hash required");
}
}

private static void checkNextArg(ArgType argType, int nextArgumentIndex) {
switch (argType) {
Original file line number Diff line number Diff line change
@@ -1785,7 +1785,7 @@ public DynamicObject formatCached(
Object[] arguments,
@Cached("privatizeRope(format)") Rope cachedFormat,
@Cached("ropeLength(cachedFormat)") int cachedFormatLength,
@Cached("create(compileFormat(format))") DirectCallNode callPackNode) {
@Cached("create(compileFormat(format, arguments))") DirectCallNode callPackNode) {
final BytesResult result;

try {
@@ -1808,7 +1808,7 @@ public DynamicObject formatUncached(
final BytesResult result;

try {
result = (BytesResult) callPackNode.call(frame, compileFormat(format),
result = (BytesResult) callPackNode.call(frame, compileFormat(format, arguments),
new Object[]{ arguments, arguments.length });
} catch (FormatException e) {
exceptionProfile.enter();
@@ -1849,12 +1849,12 @@ private DynamicObject finishFormat(int formatLength, BytesResult result) {
}

@TruffleBoundary
protected CallTarget compileFormat(DynamicObject format) {
protected CallTarget compileFormat(DynamicObject format, Object[] arguments) {
assert RubyGuards.isRubyString(format);

try {
return new PrintfCompiler(getContext(), this)
.compile(Layouts.STRING.getRope(format).getBytes());
.compile(Layouts.STRING.getRope(format).getBytes(), arguments);
} catch (InvalidFormatException e) {
throw new RaiseException(coreExceptions().argumentError(e.getMessage(), this));
}

4 comments on commit 4fb3526

@bjfish
Copy link
Contributor

@bjfish bjfish commented on 4fb3526 Dec 12, 2016

Choose a reason for hiding this comment

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

I think I may need to find another way to do argument validation to handle the cached sprintf specialization

@eregon
Copy link
Member

@eregon eregon commented on 4fb3526 Dec 12, 2016

Choose a reason for hiding this comment

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

It might be reasonable/easier to check if the key is present when executing the %<>, since that's the place where it gets problematic if the value is not provided. I don't think there is any user difference.
OTOH that wouldn;t work if the Hash needs some coercion.
Another way would be to add a check in the guards, since you can't really cache arguments easily.

@bjfish
Copy link
Contributor

@bjfish bjfish commented on 4fb3526 Dec 12, 2016

Choose a reason for hiding this comment

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

@eregon The spec is tricky because it's not even a valid format string %<foo> but this argument validation error happens mid-parsing.

@eregon
Copy link
Member

@eregon eregon commented on 4fb3526 Dec 12, 2016

Choose a reason for hiding this comment

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

Can the user tell if the error is mid-parsing or mid-formatting?

Please sign in to comment.