Skip to content

Commit

Permalink
[Truffle] Findbugs.
Browse files Browse the repository at this point in the history
@chrisseaton
chrisseaton committed Jan 13, 2015
1 parent 7e82e91 commit 3758c49
Showing 10 changed files with 41 additions and 12 deletions.
2 changes: 2 additions & 0 deletions core/src/main/java/org/jruby/truffle/nodes/methods/AddMethodNode.java
View file
Original file line number Diff line number Diff line change
@@ -79,11 +79,13 @@ private Visibility getVisibility(VirtualFrame frame, String name) {
}

return Truffle.getRuntime().iterateFrames(new FrameInstanceVisitor<Visibility>() {

@Override
public Visibility visitFrame(FrameInstance frameInstance) {
Frame frame = frameInstance.getFrame(FrameAccess.READ_ONLY, true);
return findVisibility(frame);
}

This comment has been minimized.

Sign in to view

Copy link
@eregon

eregon Jan 13, 2015

Member

Was that signaled by Findbugs?
Which spacing style should we use for these which should be lambdas in Java 8? (I have seen everything)
My personal preference goes to no space around so it looks closer to a lambda.

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jan 13, 2015

Author Contributor

No this wasn't Findbugs (which operates on classfiles, not source). Findbugs suggested to make this into a static anonymous class and to make a singleton instance. I started doing that, but decided I was doing nothing of benefit, so put it back, and must have put these lines in. I always put blank lines but I'm not massively bothered.

At some point we should add add support for Checkstyle and enforce a style guide.
});
}
}
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/truffle/nodes/supercall/AbstractGeneralSuperCallNode.java
View file
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ public AbstractGeneralSuperCallNode(RubyContext context, SourceSection sourceSec

protected boolean guard() {
// TODO(CS): not sure this is enough... lots of 'unspecified' behaviour in the ISO spec here
return method != null && unmodifiedAssumption.isValid();
return method != null && unmodifiedAssumption != null && unmodifiedAssumption.isValid();
}

protected void lookup(VirtualFrame frame) {
@@ -66,7 +66,7 @@ protected void lookup(VirtualFrame frame) {
if (method == null || method.isUndefined()) {
method = null;
// TODO: should add " for #{receiver.inspect}" in error message
throw new RaiseException(getContext().getCoreLibrary().noMethodError("super: no superclass method `"+name+"'", this));
throw new RaiseException(getContext().getCoreLibrary().noMethodError(String.format("super: no superclass method `%s'", name), this));
}

final DirectCallNode newCallNode = Truffle.getRuntime().createDirectCallNode(method.getCallTarget());
@@ -98,7 +98,7 @@ public Object isDefined(VirtualFrame frame) {
} else {
return context.makeString("super");
}
} catch (Exception e) {
} catch (Throwable t) {
return getContext().getCoreLibrary().getNilObject();
}
}
4 changes: 4 additions & 0 deletions core/src/main/java/org/jruby/truffle/nodes/supercall/GeneralSuperCallNode.java
View file
Original file line number Diff line number Diff line change
@@ -79,6 +79,10 @@ public final Object execute(VirtualFrame frame) {
lookup(frame);
}

if (method == null || callNode == null) {
throw new IllegalStateException("No call node installed");
}

// Call the method

if (isSplatted) {
4 changes: 4 additions & 0 deletions core/src/main/java/org/jruby/truffle/nodes/supercall/GeneralSuperReCallNode.java
View file
Original file line number Diff line number Diff line change
@@ -31,6 +31,10 @@ public final Object execute(VirtualFrame frame) {
lookup(frame);
}

if (method == null || callNode == null) {
throw new IllegalStateException("No call node installed");
}

final Object[] superArguments;

if (inBlock) {
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/truffle/runtime/backtrace/DebugBacktraceFormatter.java
View file
Original file line number Diff line number Diff line change
@@ -103,7 +103,7 @@ public static String debugString(RubyContext context, Object value) {
} else {
return string.substring(0, Options.TRUFFLE_BACKTRACE_MAX_VALUE_LENGTH.load()) + "…";
}
} catch (Exception e) {
} catch (Throwable t) {
return "*error*";
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/truffle/runtime/subsystems/ObjectSpaceManager.java
View file
Original file line number Diff line number Diff line change
@@ -154,7 +154,7 @@ private static void runFinalizers(FinalizerReference finalizerReference) {
for (RubyProc proc : finalizerReference.getFinalizers()) {
proc.rootCall();
}
} catch (Exception e) {
} catch (Throwable t) {
// MRI seems to silently ignore exceptions in finalizers
}
}
4 changes: 1 addition & 3 deletions core/src/main/java/org/jruby/truffle/translator/BodyTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -2456,9 +2456,7 @@ public RubyNode visitUntilNode(org.jruby.ast.UntilNode node) {
@Override
public RubyNode visitVCallNode(org.jruby.ast.VCallNode node) {
final org.jruby.ast.Node receiver = new org.jruby.ast.SelfNode(node.getPosition());
final org.jruby.ast.Node args = null;
final CallNode callNode = new CallNode(node.getPosition(), receiver, node.getName(), args, null);

final CallNode callNode = new CallNode(node.getPosition(), receiver, node.getName(), null, null);
return visitCallNodeExtraArgument(callNode, null, true, true);
}

8 changes: 8 additions & 0 deletions core/src/main/java/org/jruby/truffle/translator/LoadArgumentsTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -227,6 +227,10 @@ public RubyNode visitRestArgNode(org.jruby.ast.RestArgNode node) {

final RubyNode readNode;

if (argsNode == null) {
throw new IllegalStateException("No arguments node visited");
}

int from = argsNode.getPreCount() + argsNode.getOptionalArgsCount();
int to = -argsNode.getPostCount();
if (useArray()) {
@@ -282,6 +286,10 @@ private RubyNode translateLocalAssignment(ISourcePosition sourcePosition, String
// Optional argument
final RubyNode defaultValue = valueNode.accept(this);

if (argsNode == null) {
throw new IllegalStateException("No arguments node visited");
}

int minimum = index + 1 + argsNode.getPostCount();

if (argsNode.hasKwargs()) {
6 changes: 2 additions & 4 deletions core/src/main/java/org/jruby/truffle/translator/TranslatorDriver.java
View file
Original file line number Diff line number Diff line change
@@ -47,10 +47,8 @@ public static enum ParserContext {
private long nextReturnID = 0;

public RubyNode parse(RubyContext context, org.jruby.ast.Node parseTree, org.jruby.ast.ArgsNode argsNode, org.jruby.ast.Node bodyNode, RubyNode currentNode) {
final SourceSection sourceSection = null;

final LexicalScope lexicalScope = context.getRootLexicalScope(); // TODO(eregon): figure out how to get the lexical scope from JRuby
final SharedMethodInfo sharedMethod = new SharedMethodInfo(sourceSection, lexicalScope, "(unknown)", false, parseTree, false);
final SharedMethodInfo sharedMethod = new SharedMethodInfo(null, lexicalScope, "(unknown)", false, parseTree, false);

final TranslatorEnvironment environment = new TranslatorEnvironment(
context, environmentForFrame(context, null), this, allocateReturnID(), true, true, sharedMethod, sharedMethod.getName(), false);
@@ -65,7 +63,7 @@ public RubyNode parse(RubyContext context, org.jruby.ast.Node parseTree, org.jru
throw new RuntimeException(e);
}

return translator.compileFunctionNode(sourceSection, "(unknown)", argsNode, bodyNode, sharedMethod);
return translator.compileFunctionNode(null, "(unknown)", argsNode, bodyNode, sharedMethod);
}

public RubyRootNode parse(RubyContext context, Source source, Encoding defaultEncoding, ParserContext parserContext, MaterializedFrame parentFrame, RubyNode currentNode, NodeWrapper wrapper) {
15 changes: 15 additions & 0 deletions tool/truffle-findbugs-exclude.xml
View file
Original file line number Diff line number Diff line change
@@ -51,4 +51,19 @@
<Class name="org.jruby.truffle.nodes.core.KernelNodes$BacktickNode" />
<Bug pattern="OS_OPEN_STREAM" />
</Match>
<Match>
<Class name="org.jruby.truffle.nodes.core.FixnumNodes$PowNode" />
<Bug pattern="FE_FLOATING_POINT_EQUALITY" />
</Match>
<Match>
<Class name="org.jruby.truffle.nodes.core.FloatNodes$PowNode" />
<Bug pattern="FE_FLOATING_POINT_EQUALITY" />
</Match>
<Match>
<Class name="org.jruby.truffle.runtime.util.ArrayUtils" />
<Bug pattern="FE_FLOATING_POINT_EQUALITY" />
</Match>
<Match>
<Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON" />
</Match>
</FindBugsFilter>

0 comments on commit 3758c49

Please sign in to comment.