Remove visibility check when looking up constant with lexical scope

Author: bjfish

spec/ruby/language/constants_spec.rb

@@ -551,11 +551,24 @@ class PrivateClass
     end
 
     it "can be accessed from classes that include the module" do
-      ConstantVisibility::PrivConstModuleChild.new.private_constant_from_include.should be_true
+      ConstantVisibility::ClassIncludingPrivConstModule.new.private_constant_from_include.should be_true
+    end
+
+    it "can be accessed from modules that include the module" do
+      ConstantVisibility::ModuleIncludingPrivConstModule.private_constant_from_include.should be_true
+    end
+
+    it "raises a NameError when accessed directly from modules that include the module" do
+      -> do
+        ConstantVisibility::ModuleIncludingPrivConstModule.private_constant_self_from_include
+      end.should raise_error(NameError)
+      -> do
+        ConstantVisibility::ModuleIncludingPrivConstModule.private_constant_named_from_include
+      end.should raise_error(NameError)
     end
 
     it "is defined? from classes that include the module" do
-      ConstantVisibility::PrivConstModuleChild.new.defined_from_include.should == "constant"
+      ConstantVisibility::ClassIncludingPrivConstModule.new.defined_from_include.should == "constant"
     end
   end
 
@@ -673,7 +686,7 @@ class PrivateClass
       }
 
       -> do
-        ConstantVisibility::PrivConstModuleChild::PRIVATE_CONSTANT_MODULE
+        ConstantVisibility::ClassIncludingPrivConstModule::PRIVATE_CONSTANT_MODULE
       end.should raise_error(NameError) {|e|
         e.receiver.should == ConstantVisibility::PrivConstModule
         e.name.should == :PRIVATE_CONSTANT_MODULE

spec/ruby/language/fixtures/constant_visibility.rb

@@ -65,7 +65,7 @@ def self.defined_from_scope
     end
   end
 
-  class PrivConstModuleChild
+  class ClassIncludingPrivConstModule
     include PrivConstModule
 
     def private_constant_from_include
@@ -77,6 +77,22 @@ def defined_from_include
     end
   end
 
+  module ModuleIncludingPrivConstModule
+    include PrivConstModule
+
+    def self.private_constant_from_include
+      PRIVATE_CONSTANT_MODULE
+    end
+
+    def self.private_constant_self_from_include
+      self::PRIVATE_CONSTANT_MODULE
+    end
+
+    def self.private_constant_named_from_include
+      PrivConstModule::PRIVATE_CONSTANT_MODULE
+    end
+  end
+
   class PrivConstClassChild < PrivConstClass
     def private_constant_from_subclass
       PRIVATE_CONSTANT_CLASS

src/main/java/org/truffleruby/language/constants/LookupConstantWithDynamicScopeNode.java

@@ -15,7 +15,6 @@
 import org.truffleruby.core.module.RubyModule;
 import org.truffleruby.language.LexicalScope;
 import org.truffleruby.language.RubyConstant;
-import org.truffleruby.language.control.RaiseException;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Cached;
@@ -45,13 +44,7 @@ public RubyConstant lookupConstant(LexicalScope lexicalScope, RubyModule module,
             limit = "getCacheLimit()")
     protected RubyConstant lookupConstant(LexicalScope lexicalScope,
             @Cached("lexicalScope") LexicalScope cachedLexicalScope,
-            @Cached("doLookup(cachedLexicalScope)") ConstantLookupResult constant,
-            @Cached("isVisible(cachedLexicalScope, constant)") boolean isVisible) {
-        if (!isVisible) {
-            throw new RaiseException(
-                    getContext(),
-                    coreExceptions().nameErrorPrivateConstant(constant.getConstant().getDeclaringModule(), name, this));
-        }
+            @Cached("doLookup(cachedLexicalScope)") ConstantLookupResult constant) {
         if (constant.isDeprecated()) {
             warnDeprecatedConstant(constant.getConstant().getDeclaringModule(), constant.getConstant(), name);
         }
@@ -60,14 +53,8 @@ protected RubyConstant lookupConstant(LexicalScope lexicalScope,
 
     @Specialization
     protected RubyConstant lookupConstantUncached(LexicalScope lexicalScope,
-            @Cached ConditionProfile isVisibleProfile,
             @Cached ConditionProfile isDeprecatedProfile) {
         final ConstantLookupResult constant = doLookup(lexicalScope);
-        if (isVisibleProfile.profile(!isVisible(lexicalScope, constant))) {
-            throw new RaiseException(
-                    getContext(),
-                    coreExceptions().nameErrorPrivateConstant(constant.getConstant().getDeclaringModule(), name, this));
-        }
         if (isDeprecatedProfile.profile(constant.isDeprecated())) {
             warnDeprecatedConstant(constant.getConstant().getDeclaringModule(), constant.getConstant(), name);
         }
@@ -79,8 +66,4 @@ protected ConstantLookupResult doLookup(LexicalScope lexicalScope) {
         return ModuleOperations.lookupConstantWithLexicalScope(getContext(), lexicalScope, name);
     }
 
-    protected boolean isVisible(LexicalScope lexicalScope, ConstantLookupResult constant) {
-        return constant.isVisibleTo(getContext(), lexicalScope, lexicalScope.getLiveModule());
-    }
-
 }

src/main/java/org/truffleruby/language/constants/LookupConstantWithLexicalScopeNode.java

@@ -15,7 +15,6 @@
 import org.truffleruby.core.module.RubyModule;
 import org.truffleruby.language.LexicalScope;
 import org.truffleruby.language.RubyConstant;
-import org.truffleruby.language.control.RaiseException;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Cached;
@@ -48,11 +47,7 @@ public RubyConstant lookupConstant(LexicalScope lexicalScope, RubyModule module,
 
     @Specialization(assumptions = "constant.getAssumptions()")
     protected RubyConstant lookupConstant(
-            @Cached("doLookup()") ConstantLookupResult constant,
-            @Cached("isVisible(constant)") boolean isVisible) {
-        if (!isVisible) {
-            throw new RaiseException(getContext(), coreExceptions().nameErrorPrivateConstant(getModule(), name, this));
-        }
+            @Cached("doLookup()") ConstantLookupResult constant) {
         if (constant.isDeprecated()) {
             warnDeprecatedConstant(getModule(), constant.getConstant(), name);
         }
@@ -61,12 +56,8 @@ protected RubyConstant lookupConstant(
 
     @Specialization
     protected RubyConstant lookupConstantUncached(
-            @Cached ConditionProfile isVisibleProfile,
             @Cached ConditionProfile isDeprecatedProfile) {
         final ConstantLookupResult constant = doLookup();
-        if (isVisibleProfile.profile(!isVisible(constant))) {
-            throw new RaiseException(getContext(), coreExceptions().nameErrorPrivateConstant(getModule(), name, this));
-        }
         if (isDeprecatedProfile.profile(constant.isDeprecated())) {
             warnDeprecatedConstant(getModule(), constant.getConstant(), name);
         }
@@ -78,8 +69,4 @@ protected ConstantLookupResult doLookup() {
         return ModuleOperations.lookupConstantWithLexicalScope(getContext(), lexicalScope, name);
     }
 
-    protected boolean isVisible(ConstantLookupResult constant) {
-        return constant.isVisibleTo(getContext(), lexicalScope, getModule());
-    }
-
 }