Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: bccbfd0b296d
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: b2321a8e8a83
Choose a head ref
  • 3 commits
  • 4 files changed
  • 1 contributor

Commits on May 20, 2016

  1. Copy the full SHA
    afd1906 View commit details
  2. Fix my set mistake.

    headius committed May 20, 2016
    Copy the full SHA
    6bea316 View commit details
  3. Merge pull request #3906 from headius/fix_kwargs_scoping

    Kwargs should not need dynamic scope, since we have static handy.
    headius committed May 20, 2016
    Copy the full SHA
    b2321a8 View commit details
19 changes: 12 additions & 7 deletions core/src/main/java/org/jruby/ir/IRScope.java
Original file line number Diff line number Diff line change
@@ -662,14 +662,19 @@ private void calculateClosureScopeFlags() {
}
}

private static final EnumSet<IRFlags> NEEDS_DYNAMIC_SCOPE_FLAGS =
EnumSet.of(
CAN_RECEIVE_BREAKS,
HAS_NONLOCAL_RETURNS,CAN_RECEIVE_NONLOCAL_RETURNS,
BINDING_HAS_ESCAPED,
USES_ZSUPER);

private void computeNeedsDynamicScopeFlag() {
// SSS FIXME: checkArity for keyword args looks up a keyword arg in the static scope which
// currently requires a dynamic scope to be recovered. If there is another way to do this,
// we can get rid of this.
if (flags.contains(CAN_RECEIVE_BREAKS) || flags.contains(HAS_NONLOCAL_RETURNS) ||
flags.contains(CAN_RECEIVE_NONLOCAL_RETURNS) || flags.contains(BINDING_HAS_ESCAPED) ||
flags.contains(USES_ZSUPER) || flags.contains(RECEIVES_KEYWORD_ARGS)) {
flags.add(REQUIRES_DYNSCOPE);
for (IRFlags f : NEEDS_DYNAMIC_SCOPE_FLAGS) {
if (flags.contains(f)) {
flags.add(REQUIRES_DYNSCOPE);
return;
}
}
}

Original file line number Diff line number Diff line change
@@ -66,7 +66,7 @@ public static CheckArityInstr decode(IRReaderDecoder d) {
}

public void checkArity(ThreadContext context, Object[] args, Block.Type blockType) {
IRRuntimeHelpers.checkArity(context, args, required, opt, rest, receivesKeywords, restKey, blockType);
IRRuntimeHelpers.checkArity(context.runtime, context.getCurrentStaticScope(), args, required, opt, rest, receivesKeywords, restKey, blockType);
}

@Override
14 changes: 6 additions & 8 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Original file line number Diff line number Diff line change
@@ -535,12 +535,12 @@ public static Block getBlockFromObject(ThreadContext context, Object value) {
return block;
}

public static void checkArity(ThreadContext context, Object[] args, int required, int opt, boolean rest,
public static void checkArity(Ruby runtime, StaticScope scope, Object[] args, int required, int opt, boolean rest,
boolean receivesKwargs, int restKey, Block.Type blockType) {
int argsLength = args.length;
RubyHash keywordArgs = extractKwargsHash(args, required, receivesKwargs);

if (restKey == -1 && keywordArgs != null) checkForExtraUnwantedKeywordArgs(context, keywordArgs);
if (restKey == -1 && keywordArgs != null) checkForExtraUnwantedKeywordArgs(runtime, scope, keywordArgs);

// keyword arguments value is not used for arity checking.
if (keywordArgs != null) argsLength -= 1;
@@ -549,7 +549,7 @@ public static void checkArity(ThreadContext context, Object[] args, int required
// System.out.println("NUMARGS: " + argsLength + ", REQUIRED: " + required + ", OPT: " + opt + ", AL: " + args.length + ",RKW: " + receivesKwargs );
// System.out.println("ARGS[0]: " + args[0]);

Arity.raiseArgumentError(context.runtime, argsLength, required, required + opt);
Arity.raiseArgumentError(runtime, argsLength, required, required + opt);
}
}

@@ -588,19 +588,17 @@ public static RubyHash extractKwargsHash(Object[] args, int requiredArgsCount, b
return null;
}

public static void checkForExtraUnwantedKeywordArgs(final ThreadContext context, RubyHash keywordArgs) {
final StaticScope scope = context.getCurrentStaticScope();

public static void checkForExtraUnwantedKeywordArgs(final Ruby runtime, final StaticScope scope, RubyHash keywordArgs) {
keywordArgs.visitAll(new RubyHash.Visitor() {
@Override
public void visit(IRubyObject key, IRubyObject value) {
String keyAsString = key.asJavaString();
int slot = scope.isDefined((keyAsString));

// Found name in higher variable scope. Therefore non for this block/method def.
if ((slot >> 16) > 0) throw context.runtime.newArgumentError("unknown keyword: " + keyAsString);
if ((slot >> 16) > 0) throw runtime.newArgumentError("unknown keyword: " + keyAsString);
// Could not find it anywhere.
if (((short) (slot & 0xffff)) < 0) throw context.runtime.newArgumentError("unknown keyword: " + keyAsString);
if (((short) (slot & 0xffff)) < 0) throw runtime.newArgumentError("unknown keyword: " + keyAsString);
}
});
}
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -968,15 +968,16 @@ public void CheckArityInstr(CheckArityInstr checkarityinstr) {
if (jvm.methodData().specificArity >= 0) {
// no arity check in specific arity path
} else {
jvmMethod().loadContext();
jvmMethod().loadRuntime();
jvmMethod().loadStaticScope();
jvmMethod().loadArgs();
jvmAdapter().ldc(checkarityinstr.required);
jvmAdapter().ldc(checkarityinstr.opt);
jvmAdapter().ldc(checkarityinstr.rest);
jvmAdapter().ldc(checkarityinstr.receivesKeywords);
jvmAdapter().ldc(checkarityinstr.restKey);
jvmMethod().loadBlockType();
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkArity", sig(void.class, ThreadContext.class, Object[].class, int.class, int.class, boolean.class, boolean.class, int.class, Block.Type.class));
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "checkArity", sig(void.class, Ruby.class, StaticScope.class, Object[].class, int.class, int.class, boolean.class, boolean.class, int.class, Block.Type.class));
}
}