From 7cc9b76a3cc434abb3e87fe026b8d8de39695431 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Fri, 15 Oct 2021 18:33:34 +0300 Subject: [PATCH 1/6] Don't warn when uninitialized instance variable is accessed --- .../objects/ReadInstanceVariableNode.java | 19 ------------------- .../truffleruby/parser/BodyTranslator.java | 2 -- 2 files changed, 21 deletions(-) diff --git a/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java b/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java index 0e997825a14a..49a8d2172c5b 100644 --- a/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java +++ b/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java @@ -45,20 +45,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); @@ -87,15 +79,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()); - } - } - } diff --git a/src/main/java/org/truffleruby/parser/BodyTranslator.java b/src/main/java/org/truffleruby/parser/BodyTranslator.java index 414e98ce6f57..467a465f58b9 100644 --- a/src/main/java/org/truffleruby/parser/BodyTranslator.java +++ b/src/main/java/org/truffleruby/parser/BodyTranslator.java @@ -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); From 4850704e7fec94b90861735945c1f30f4a95c416 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Fri, 15 Oct 2021 23:19:53 +0300 Subject: [PATCH 2/6] Remove not used declaration and imports --- .../truffleruby/language/objects/ReadInstanceVariableNode.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java b/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java index 49a8d2172c5b..7b427967724d 100644 --- a/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java +++ b/src/main/java/org/truffleruby/language/objects/ReadInstanceVariableNode.java @@ -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; @@ -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(); From d27d0b3cc19275a26a7043410165df99b71376ad Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Fri, 15 Oct 2021 23:22:51 +0300 Subject: [PATCH 3/6] Add spec for warning emitting about accessing an uninitialized instance variable --- spec/ruby/language/variables_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/ruby/language/variables_spec.rb b/spec/ruby/language/variables_spec.rb index 7d6969e65937..7975ca60b33e 100644 --- a/spec/ruby/language/variables_spec.rb +++ b/spec/ruby/language/variables_spec.rb @@ -822,3 +822,25 @@ 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 + end +end From 8b34fa6eb0f2335e0084c42cc818c27e6ec93b07 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Sun, 17 Oct 2021 01:59:11 +0300 Subject: [PATCH 4/6] Skip definition check for InstVarParseNode in "||=" operation --- .../java/org/truffleruby/parser/ast/InstVarParseNode.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/truffleruby/parser/ast/InstVarParseNode.java b/src/main/java/org/truffleruby/parser/ast/InstVarParseNode.java index 669cd6993453..80f827ccf9ff 100644 --- a/src/main/java/org/truffleruby/parser/ast/InstVarParseNode.java +++ b/src/main/java/org/truffleruby/parser/ast/InstVarParseNode.java @@ -80,4 +80,9 @@ public List childNodes() { public void setName(String name) { this.name = name; } + + @Override + public boolean needsDefinitionCheck() { + return false; + } } From f9b0bf25dfaa9ced6a8587579d6cdeeb2122b905 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Mon, 18 Oct 2021 12:56:53 +0300 Subject: [PATCH 5/6] Add test about no warnings emitting at lazy initialization of an instance variable --- spec/ruby/language/variables_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/ruby/language/variables_spec.rb b/spec/ruby/language/variables_spec.rb index 7975ca60b33e..699187335c18 100644 --- a/spec/ruby/language/variables_spec.rb +++ b/spec/ruby/language/variables_spec.rb @@ -842,5 +842,12 @@ 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 From 6833ee9cad1c3268c162e524326c3c69c5854faa Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 21 Oct 2021 15:09:01 +0200 Subject: [PATCH 6/6] Add ChangeLog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3abbd17d041d..02c329534657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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: