Skip to content

[Ruby 3.0 support] Don't warn when uninitialized instance variable is accessed #2502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Compatibility:
* `Module#attr_*` methods now return an array of method names (#2498, @gogainda).
* Fixed `Socket#(local|remote)_address` to retrieve family and type from the file descriptor (#2444, @larskanis).
* Add `Thread.ignore_deadlock` accessor (#2453).
* Do not warn when uninitialized instance variable is accessed (#2502, @andrykonchin).

Performance:

Expand Down
29 changes: 29 additions & 0 deletions spec/ruby/language/variables_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -822,3 +822,32 @@ def test
result.should == 1
end
end

describe "Instance variables" do
context "when instance variable is uninitialized" do
ruby_version_is ""..."3.0" do
it "warns about accessing uninitialized instance variable" do
obj = Object.new
def obj.foobar; a = @a; end

-> { obj.foobar }.should complain(/warning: instance variable @a not initialized/, verbose: true)
end
end

ruby_version_is "3.0" do
it "doesn't warn about accessing uninitialized instance variable" do
obj = Object.new
def obj.foobar; a = @a; end

-> { obj.foobar }.should_not complain(verbose: true)
end
end

it "doesn't warn at lazy initialization" do
obj = Object.new
def obj.foobar; @a ||= 42; end

-> { obj.foobar }.should_not complain(verbose: true)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
import org.truffleruby.language.RubyContextSourceNode;
import org.truffleruby.language.RubyDynamicObject;
import org.truffleruby.language.RubyNode;
import org.truffleruby.language.WarningNode;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.object.DynamicObjectLibrary;
import com.oracle.truffle.api.profiles.ConditionProfile;
Expand All @@ -29,7 +27,6 @@ public class ReadInstanceVariableNode extends RubyContextSourceNode {

@Child private RubyNode receiver;
@Child private DynamicObjectLibrary objectLibrary;
@Child private WarningNode warningNode;

private final ConditionProfile objectProfile = ConditionProfile.create();

Expand All @@ -45,20 +42,12 @@ public Object execute(VirtualFrame frame) {
if (objectProfile.profile(receiverObject instanceof RubyDynamicObject)) {
final DynamicObjectLibrary objectLibrary = getObjectLibrary();
final RubyDynamicObject dynamicObject = (RubyDynamicObject) receiverObject;
if (!objectLibrary.containsKey(dynamicObject, name)) {
warnNotInitialized();
}
return objectLibrary.getOrDefault(dynamicObject, name, nil);
} else {
return nil;
}
}

@TruffleBoundary
private String getWarningMessage() {
return String.format("instance variable %s not initialized", name);
}

@Override
public Object isDefined(VirtualFrame frame, RubyLanguage language, RubyContext context) {
final Object receiverObject = receiver.execute(frame);
Expand Down Expand Up @@ -87,15 +76,4 @@ private DynamicObjectLibrary getObjectLibrary() {
return objectLibrary;
}

private void warnNotInitialized() {
if (warningNode == null) {
CompilerDirectives.transferToInterpreterAndInvalidate();
warningNode = insert(new WarningNode());
}

if (warningNode.shouldWarn()) {
warningNode.warningMessage(getSourceSection(), getWarningMessage());
}
}

}
2 changes: 0 additions & 2 deletions src/main/java/org/truffleruby/parser/BodyTranslator.java
Original file line number Diff line number Diff line change
Expand Up @@ -2424,8 +2424,6 @@ public RubyNode visitOpAsgnOrNode(OpAsgnOrParseNode node) {
RubyNode rhs = node.getSecondNode().accept(this);

// This is needed for class variables. Constants are handled separately in visitOpAsgnConstDeclNode.
// It is also needed for instance variables to prevent attempted read which may trigger "not initialized"
// warnings unintentionally.
if (node.getFirstNode().needsDefinitionCheck()) {
RubyNode defined = new DefinedNode(lhs);
lhs = new AndNode(defined, lhs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,9 @@ public List<ParseNode> childNodes() {
public void setName(String name) {
this.name = name;
}

@Override
public boolean needsDefinitionCheck() {
return false;
}
}