-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Truffle] Sprintf check for too many args when $DEBUG is true #4380
Conversation
@@ -255,6 +264,13 @@ public PrintfSimpleParser(char[] source, Object[] arguments) { | |||
} | |||
} | |||
} | |||
if((argType == ArgType.UNNUMBERED || argType == ArgType.NONE) && | |||
arguments.length > argCount ){ | |||
if(isDebug){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spacing around here looks inconsistent with the rest.
} catch (InvalidFormatException e) { | ||
throw new RaiseException(coreExceptions().argumentError(e.getMessage(), this)); | ||
} | ||
} | ||
|
||
protected boolean isDebug(VirtualFrame frame) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use a BooleanCastNode, using UnexpectedResultException directly is usually bad style.
MRI behaves with $DEBUG
as 42 the same as true, and with nil the same as false.
@@ -1774,18 +1776,25 @@ public static long sleepFor(Node currentNode, RubyContext context, final long du | |||
|
|||
@Child private RopeNodes.MakeLeafRopeNode makeLeafRopeNode; | |||
@Child private TaintNode taintNode; | |||
@Child private ReadGlobalVariableNode readDebugGlobalNode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BooleanCastNodeGen.create(ReadGlobalVariableNodeGen.create("$DEBUG"));
looks like a good way.
You need to remove the context and section from ReadGlobalVariableNode's constructor, which are not used anyway.
Looks good overall! |
1c9c38e
to
e57a242
Compare
e57a242
to
b21ffa5
Compare
} catch (InvalidFormatException e) { | ||
throw new RaiseException(coreExceptions().argumentError(e.getMessage(), this)); | ||
} | ||
} | ||
|
||
protected boolean isDebug(VirtualFrame frame) { | ||
try { | ||
return readDebugGlobalNode.executeBoolean(frame); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the right node/way equivalent of RTEST(ruby_debug)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BooleanCastNode. executeBoolean(frame, value)
.
Opening PR for feedback