Skip to content

Commit

Permalink
[Truffle] Findbugs.
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrisseaton
chrisseaton committed Jan 13, 2015
1 parent 7e82e91 commit 3758c49
Show file tree
Hide file tree
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
Expand Up @@ -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.
});
}
}
Expand Down
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
Expand Up @@ -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) {
Expand All @@ -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());
Expand Down Expand Up @@ -98,7 +98,7 @@ public Object isDefined(VirtualFrame frame) {
} else {
return context.makeString("super");
}
} catch (Exception e) {
} catch (Throwable t) {
return getContext().getCoreLibrary().getNilObject();
}
}
Expand Down
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
Expand Up @@ -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) {
Expand Down
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
Expand Up @@ -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) {
Expand Down
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
Expand Up @@ -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*";
}
}
Expand Down
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
Expand Up @@ -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
}
}
Expand Down
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
Expand Up @@ -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);
}

Expand Down
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
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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()) {
Expand Down
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
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions tool/truffle-findbugs-exclude.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.