Skip to content

Commit f4af1b2

Browse files
committed
Use the class where the method was defined to check if an UnboundMethod can be used for Module#define_method
* Let canBindMethodTo() takes an InternalMethod to ensure we always use getDeclaringModule().
1 parent 09794e2 commit f4af1b2

File tree

5 files changed

+16
-15
lines changed

5 files changed

+16
-15
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Bug fixes:
1010
* Fixed `Symbol#match` returning `MatchData` (#1706).
1111
* Allow `Time#strftime` to be called with binary format strings.
1212
* Do not modify the argument passed to `IO#write` when the encoding does not match (#1714).
13+
* Use the class where the method was defined to check if an `UnboundMethod` can be used for `#define_method` (#1710).
1314

1415
Compatibility:
1516

src/main/java/org/truffleruby/core/module/ModuleFields.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ public RubyConstant removeConstant(RubyContext context, Node currentNode, String
363363

364364
@TruffleBoundary
365365
public void addMethod(RubyContext context, Node currentNode, InternalMethod method) {
366-
assert ModuleOperations.canBindMethodTo(method.getDeclaringModule(), rubyModuleObject) ||
366+
assert ModuleOperations.canBindMethodTo(method, rubyModuleObject) ||
367367
ModuleOperations.assignableTo(context.getCoreLibrary().getObjectClass(), method.getDeclaringModule()) ||
368368
// TODO (pitr-ch 24-Jul-2016): find out why undefined methods sometimes do not match above assertion
369369
// e.g. "block in _routes route_set.rb:525" in rails/actionpack/lib/action_dispatch/routing/

src/main/java/org/truffleruby/core/module/ModuleNodes.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,14 +1049,15 @@ public DynamicObject defineMethodMethod(DynamicObject module, String name, Dynam
10491049
@TruffleBoundary
10501050
@Specialization(guards = "isRubyUnboundMethod(method)")
10511051
public DynamicObject defineMethod(DynamicObject module, String name, DynamicObject method, NotProvided block) {
1052-
final DynamicObject origin = Layouts.UNBOUND_METHOD.getOrigin(method);
1053-
if (!ModuleOperations.canBindMethodTo(origin, module)) {
1054-
throw new RaiseException(getContext(), coreExceptions().typeError("bind argument must be a subclass of " + Layouts.MODULE.getFields(origin).getName(), this));
1052+
final InternalMethod internalMethod = Layouts.UNBOUND_METHOD.getMethod(method);
1053+
if (!ModuleOperations.canBindMethodTo(internalMethod, module)) {
1054+
final DynamicObject declaringModule = internalMethod.getDeclaringModule();
1055+
throw new RaiseException(getContext(), coreExceptions().typeError("bind argument must be a subclass of " + Layouts.MODULE.getFields(declaringModule).getName(), this));
10551056
}
10561057

10571058
// TODO CS 5-Apr-15 TypeError if the method came from a singleton
10581059

1059-
return addMethod(module, name, Layouts.UNBOUND_METHOD.getMethod(method));
1060+
return addMethod(module, name, internalMethod);
10601061
}
10611062

10621063
@TruffleBoundary

src/main/java/org/truffleruby/core/module/ModuleOperations.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,17 @@ public static boolean inAncestorsOf(DynamicObject module, DynamicObject ancestor
6060
}
6161

6262
@TruffleBoundary
63-
public static boolean canBindMethodTo(DynamicObject origin, DynamicObject module) {
64-
assert RubyGuards.isRubyModule(origin);
63+
public static boolean canBindMethodTo(InternalMethod method, DynamicObject module) {
6564
assert RubyGuards.isRubyModule(module);
65+
final DynamicObject origin = method.getDeclaringModule();
6666

67-
if (!(RubyGuards.isRubyClass(origin))) {
67+
if (!RubyGuards.isRubyClass(origin)) { // Module (not Class) methods can always be bound
6868
return true;
6969
} else if (Layouts.MODULE.getFields(module).isRefinement()) {
7070
DynamicObject refinedClass = Layouts.MODULE.getFields(module).getRefinedClass();
71-
return ((RubyGuards.isRubyClass(refinedClass)) && ModuleOperations.assignableTo(refinedClass, origin));
71+
return RubyGuards.isRubyClass(refinedClass) && ModuleOperations.assignableTo(refinedClass, origin);
7272
} else {
73-
return ((RubyGuards.isRubyClass(module)) && ModuleOperations.assignableTo(module, origin));
73+
return RubyGuards.isRubyClass(module) && ModuleOperations.assignableTo(module, origin);
7474
}
7575
}
7676

src/main/java/org/truffleruby/language/methods/CanBindMethodToModuleNode.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,17 @@ public static CanBindMethodToModuleNode create() {
3232
protected boolean canBindMethodToCached(InternalMethod method, DynamicObject module,
3333
@Cached("method.getDeclaringModule()") DynamicObject declaringModule,
3434
@Cached("module") DynamicObject cachedModule,
35-
@Cached("canBindMethodTo(declaringModule, cachedModule)") boolean canBindMethodTo) {
35+
@Cached("canBindMethodTo(method, cachedModule)") boolean canBindMethodTo) {
3636
return canBindMethodTo;
3737
}
3838

3939
@Specialization(guards = "isRubyModule(module)")
4040
protected boolean canBindMethodToUncached(InternalMethod method, DynamicObject module) {
41-
final DynamicObject declaringModule = method.getDeclaringModule();
42-
return canBindMethodTo(declaringModule, module);
41+
return canBindMethodTo(method, module);
4342
}
4443

45-
protected boolean canBindMethodTo(DynamicObject declaringModule, DynamicObject module) {
46-
return ModuleOperations.canBindMethodTo(declaringModule, module);
44+
protected boolean canBindMethodTo(InternalMethod method, DynamicObject module) {
45+
return ModuleOperations.canBindMethodTo(method, module);
4746
}
4847

4948
protected int getCacheLimit() {

0 commit comments

Comments
 (0)