Skip to content

Commit

Permalink
Record the SourceSection in the constant for the module/class keywords
Browse files Browse the repository at this point in the history
* Fixes oracle#2833
  • Loading branch information
eregon authored and john-heinnickel committed Aug 16, 2023
1 parent fd34dfe commit d820c4d
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Compatibility:
* Fix `String#casecmp?` for empty strings of different encodings (#2826, @eregon).
* Implement `Enumerable#compact` and `Enumerator::Lazy#compact` (#2733, @andrykonchin).
* Implement `Array#intersect?` (#2831, @nirvdrum).
* Record the source location in the constant for the `module`/`class` keywords (#2833, @eregon).

Performance:

Expand Down
7 changes: 6 additions & 1 deletion spec/ruby/core/module/const_source_location_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@
end

describe "with statically assigned constants" do
it "works for the module and class keywords" do
ConstantSpecs.const_source_location(:ModuleB).should == [@constants_fixture_path, ConstantSpecs::ModuleB::LINE]
ConstantSpecs.const_source_location(:ClassA).should == [@constants_fixture_path, ConstantSpecs::ClassA::LINE]
end

it "searches location path the immediate class or module first" do
ConstantSpecs::ClassA.const_source_location(:CS_CONST10).should == [@constants_fixture_path, ConstantSpecs::ClassA::CS_CONST10_LINE]
ConstantSpecs::ModuleA.const_source_location(:CS_CONST10).should == [@constants_fixture_path, ConstantSpecs::ModuleA::CS_CONST10_LINE]
Expand Down Expand Up @@ -133,7 +138,7 @@
it "calls #to_str to convert the given name to a String" do
name = mock("ClassA")
name.should_receive(:to_str).and_return("ClassA")
ConstantSpecs.const_source_location(name).should == [@constants_fixture_path, ConstantSpecs::ClassA::CS_CLASS_A_LINE]
ConstantSpecs.const_source_location(name).should == [@constants_fixture_path, ConstantSpecs::ClassA::LINE]
end

it "raises a TypeError if conversion to a String by calling #to_str fails" do
Expand Down
3 changes: 2 additions & 1 deletion spec/ruby/fixtures/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module ModuleA

# Included in ParentA
module ModuleB
LINE = __LINE__ - 1
CS_CONST10 = :const10_9
CS_CONST11 = :const11_2
CS_CONST12 = :const12_1
Expand Down Expand Up @@ -87,7 +88,7 @@ module ModuleIncludePrepended
# are run.

class ClassA
CS_CLASS_A_LINE = __LINE__ - 1
LINE = __LINE__ - 1
CS_CONST10 = :const10_10
CS_CONST10_LINE = __LINE__ - 1
CS_CONST16 = :const16
Expand Down
1 change: 0 additions & 1 deletion spec/tags/core/module/const_source_location_tags.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
fails:Module#const_source_location calls #to_str to convert the given name to a String
fails:Module#const_source_location autoload returns the autoload location while not resolved
fails:Module#const_source_location returns updated location from const_set
4 changes: 2 additions & 2 deletions src/main/java/org/truffleruby/core/CoreLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -732,11 +732,11 @@ private RubyClass defineClass(String name) {
}

private RubyClass defineClass(RubyClass superclass, String name) {
return ClassNodes.createInitializedRubyClass(context, null, objectClass, superclass, name);
return ClassNodes.createInitializedRubyClass(context, null, objectClass, superclass, name, node);
}

private RubyClass defineClass(RubyModule lexicalParent, RubyClass superclass, String name) {
return ClassNodes.createInitializedRubyClass(context, null, lexicalParent, superclass, name);
return ClassNodes.createInitializedRubyClass(context, null, lexicalParent, superclass, name, node);
}

private RubyModule defineModule(String name) {
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/org/truffleruby/core/klass/ClassNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.oracle.truffle.api.RootCallTarget;
import com.oracle.truffle.api.dsl.GenerateUncached;
import com.oracle.truffle.api.frame.Frame;
import com.oracle.truffle.api.nodes.Node;
import org.truffleruby.RubyContext;
import org.truffleruby.RubyLanguage;
import org.truffleruby.annotations.CoreMethod;
Expand Down Expand Up @@ -78,13 +79,14 @@ public static RubyClass createSingletonClassOfObject(RubyContext context, Source
superclass,
null,
true,
attached);
attached,
null);
return ensureItHasSingletonClassCreated(context, rubyClass);
}

@TruffleBoundary
public static RubyClass createInitializedRubyClass(RubyContext context, SourceSection sourceSection,
RubyModule lexicalParent, RubyClass superclass, String name) {
RubyModule lexicalParent, RubyClass superclass, String name, Node currentNode) {
assert superclass != null;
final RubyClass rubyClass = createRubyClass(
context,
Expand All @@ -94,7 +96,8 @@ public static RubyClass createInitializedRubyClass(RubyContext context, SourceSe
superclass,
name,
false,
null);
null,
currentNode);
return ensureItHasSingletonClassCreated(context, rubyClass);
}

Expand All @@ -106,7 +109,8 @@ private static RubyClass createRubyClass(RubyContext context,
RubyClass superclass,
String name,
boolean isSingleton,
RubyDynamicObject attached) {
RubyDynamicObject attached,
Node currentNode) {
assert superclass != null;
final RubyClass rubyClass = new RubyClass(
classClass,
Expand All @@ -119,7 +123,7 @@ private static RubyClass createRubyClass(RubyContext context,
superclass);

if (lexicalParent != null) {
rubyClass.fields.getAdoptedByLexicalParent(context, lexicalParent, name, null);
rubyClass.fields.getAdoptedByLexicalParent(context, lexicalParent, name, currentNode);
}

return rubyClass;
Expand Down Expand Up @@ -182,7 +186,8 @@ private static RubyClass createSingletonClass(RubyContext context, RubyClass rub
singletonSuperclass,
null,
true,
rubyClass);
rubyClass,
null);
SharedObjects.propagate(context.getLanguageSlow(), rubyClass, metaClass);
rubyClass.setMetaClass(metaClass);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public Object execute(VirtualFrame frame) {
getEncapsulatingSourceSection(),
lexicalParentModule,
superClass,
name);
name,
this);
callInherited(frame, superClass, definedClass);
} else {
if (!(existing instanceof RubyClass)) {
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/truffleruby/parser/BodyTranslator.java
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,7 @@ public RubyNode visitClassNode(ClassParseNode node) {

final RubyNode superClass = node.getSuperNode() != null ? node.getSuperNode().accept(this) : null;
final DefineClassNode defineOrGetClass = new DefineClassNode(name, lexicalParent, superClass);
defineOrGetClass.unsafeSetSourceSection(sourceSection);

final RubyNode ret = openModule(
sourceSection,
Expand Down Expand Up @@ -2199,6 +2200,7 @@ public RubyNode visitModuleNode(ModuleParseNode node) {
RubyNode lexicalParent = translateCPath(sourceSection, node.getCPath());

final DefineModuleNode defineModuleNode = DefineModuleNodeGen.create(name, lexicalParent);
defineModuleNode.unsafeSetSourceSection(sourceSection);

final RubyNode ret = openModule(
sourceSection,
Expand Down Expand Up @@ -2868,6 +2870,7 @@ public RubyNode visitSClassNode(SClassParseNode node) {

final RubyNode receiverNode = node.getReceiverNode().accept(this);
final SingletonClassNode singletonClassNode = SingletonClassNodeGen.create(receiverNode);
singletonClassNode.unsafeSetSourceSection(sourceSection);

boolean dynamicConstantLookup = environment.isDynamicConstantLookup();

Expand Down

0 comments on commit d820c4d

Please sign in to comment.