Skip to content

Commit 9a36c6e

Browse files
committed
[GR-37585] Update method owner when it is being aliased
PullRequest: truffleruby/3261
2 parents 61e87bf + 54df760 commit 9a36c6e

File tree

7 files changed

+63
-4
lines changed

7 files changed

+63
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Bug fixes:
99
* Fix `MatchData#[]` exception when passing a length argument larger than the number of match values (#2636, @nirvdrum).
1010
* Fix `MatchData#[]` exception when supplying a large negative index along with a length argument (@nirvdrum).
1111
* Fix capacity computation for huge `Hash` (#2635, @eregon).
12+
* Fix aliased methods to return the correct owner when method is from a superclass (@bjfish).
1213

1314
Compatibility:
1415

spec/ruby/language/alias_spec.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,19 @@ def test_with_check(*args)
243243
e.class.should == NameError
244244
}
245245
end
246+
247+
it "defines the method on the aliased class when the original method is from a parent class" do
248+
parent = Class.new do
249+
def parent_method
250+
end
251+
end
252+
child = Class.new(parent) do
253+
alias parent_method_alias parent_method
254+
end
255+
256+
child.instance_method(:parent_method_alias).owner.should == child
257+
child.instance_methods(false).should include(:parent_method_alias)
258+
end
246259
end
247260

248261
describe "The alias keyword" do

src/main/java/org/truffleruby/core/method/MethodNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public abstract static class OwnerNode extends CoreMethodArrayArgumentsNode {
172172

173173
@Specialization
174174
protected RubyModule owner(RubyMethod method) {
175-
return method.method.getDeclaringModule();
175+
return method.method.getOwner();
176176
}
177177

178178
}

src/main/java/org/truffleruby/core/method/UnboundMethodNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public abstract static class OwnerNode extends CoreMethodArrayArgumentsNode {
155155

156156
@Specialization
157157
protected RubyModule owner(RubyUnboundMethod unboundMethod) {
158-
return unboundMethod.method.getDeclaringModule();
158+
return unboundMethod.method.getOwner();
159159
}
160160

161161
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ public void initCopy(RubyModule from) {
212212
if (methodEntry.getMethod() != null) {
213213
MethodEntry newMethodEntry = new MethodEntry(methodEntry
214214
.getMethod()
215-
.withDeclaringModule(rubyModule));
215+
.withDeclaringModule(rubyModule)
216+
.withOwner(rubyModule));
216217
this.methods.put(methodEntry.getMethod().getName(), newMethodEntry);
217218
}
218219
}
@@ -512,6 +513,8 @@ public void addMethod(RubyContext context, Node currentNode, InternalMethod meth
512513

513514
checkFrozen(context, currentNode);
514515

516+
method = method.withOwner(rubyModule);
517+
515518
if (SharedObjects.isShared(rubyModule)) {
516519
Set<Object> adjacent = ObjectGraph.newObjectSet();
517520
ObjectGraph.addProperty(adjacent, method);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ public void addMethodIgnoreNameVisibility(RubyContext context, InternalMethod me
9696
singletonClass.fields.addMethod(
9797
context,
9898
currentNode,
99-
method.withDeclaringModule(singletonClass).withVisibility(Visibility.PUBLIC));
99+
method.withDeclaringModule(singletonClass).withOwner(singletonClass)
100+
.withVisibility(Visibility.PUBLIC));
100101
} else {
101102
fields.addMethod(context, currentNode, method.withVisibility(visibility));
102103
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ public class InternalMethod implements ObjectGraphNode {
4040
private final DeclarationContext activeRefinements;
4141
private final String name;
4242

43+
/** The module on which the method was initially declared in the source code (e.g., with def) */
4344
private final RubyModule declaringModule;
45+
/** The module owning this InternalMethod (i.e., the method is part of that module's instance methods) */
46+
private final RubyModule owner;
4447
private final Visibility visibility;
4548
private final boolean undefined;
4649
private final boolean unimplemented; // similar to MRI's rb_f_notimplement
@@ -124,6 +127,7 @@ public InternalMethod(
124127
declarationContext,
125128
name,
126129
declaringModule,
130+
declaringModule,
127131
visibility,
128132
undefined,
129133
false,
@@ -142,6 +146,7 @@ private InternalMethod(
142146
DeclarationContext declarationContext,
143147
String name,
144148
RubyModule declaringModule,
149+
RubyModule owner,
145150
Visibility visibility,
146151
boolean undefined,
147152
boolean unimplemented,
@@ -160,6 +165,7 @@ private InternalMethod(
160165
this.lexicalScope = lexicalScope;
161166
this.declarationContext = declarationContext;
162167
this.declaringModule = declaringModule;
168+
this.owner = owner;
163169
this.name = name;
164170
this.visibility = visibility;
165171
this.undefined = undefined;
@@ -181,6 +187,10 @@ public RubyModule getDeclaringModule() {
181187
return declaringModule;
182188
}
183189

190+
public RubyModule getOwner() {
191+
return owner;
192+
}
193+
184194
public String getName() {
185195
return name;
186196
}
@@ -239,6 +249,31 @@ public InternalMethod withDeclaringModule(RubyModule newDeclaringModule) {
239249
declarationContext,
240250
name,
241251
newDeclaringModule,
252+
owner,
253+
visibility,
254+
undefined,
255+
unimplemented,
256+
builtIn,
257+
alwaysInlinedNodeFactory,
258+
activeRefinements,
259+
proc,
260+
callTarget,
261+
callTargetSupplier,
262+
capturedBlock);
263+
}
264+
}
265+
266+
public InternalMethod withOwner(RubyModule newOwner) {
267+
if (newOwner == owner) {
268+
return this;
269+
} else {
270+
return new InternalMethod(
271+
sharedMethodInfo,
272+
lexicalScope,
273+
declarationContext,
274+
name,
275+
declaringModule,
276+
newOwner,
242277
visibility,
243278
undefined,
244279
unimplemented,
@@ -262,6 +297,7 @@ public InternalMethod withName(String newName) {
262297
declarationContext,
263298
newName,
264299
declaringModule,
300+
owner,
265301
visibility,
266302
undefined,
267303
unimplemented,
@@ -285,6 +321,7 @@ public InternalMethod withVisibility(Visibility newVisibility) {
285321
declarationContext,
286322
name,
287323
declaringModule,
324+
owner,
288325
newVisibility,
289326
undefined,
290327
unimplemented,
@@ -308,6 +345,7 @@ public InternalMethod withActiveRefinements(DeclarationContext context) {
308345
declarationContext,
309346
name,
310347
declaringModule,
348+
owner,
311349
visibility,
312350
undefined,
313351
unimplemented,
@@ -331,6 +369,7 @@ public InternalMethod withDeclarationContext(DeclarationContext newDeclarationCo
331369
newDeclarationContext,
332370
name,
333371
declaringModule,
372+
owner,
334373
visibility,
335374
undefined,
336375
unimplemented,
@@ -351,6 +390,7 @@ public InternalMethod undefined() {
351390
declarationContext,
352391
name,
353392
declaringModule,
393+
owner,
354394
visibility,
355395
true,
356396
unimplemented,
@@ -370,6 +410,7 @@ public InternalMethod unimplemented() {
370410
declarationContext,
371411
name,
372412
declaringModule,
413+
owner,
373414
visibility,
374415
undefined,
375416
true,

0 commit comments

Comments
 (0)