Feature #19099: Support `private_constant` for an undefined constant

• Status: Open
• Assignee: -
• Target version: -

[ujihisa (Tatsuhiro Ujihisa)]

All the following discussion applies to public_constant too. Maybe deprecate_constant as well.

Problem

class C
  X = ...
  private_constant :X
end

The above idiom usually works fine, but when ... part is long, like a 30-line Ruby Hash, it's very easy to miss the following private_constant :X part.

Impossible solution

class C
  private_constant X = ...
end

Like private, if the above notation could work, it would be awesome, but it breaks so many backward compatibility. The constant assignment returns its value but not the name of the constant, and we should keep the current behaviour.

Proposed solution

Allow the following new notation for private_constant by making constant private by name without actually resolving itself and raises an error.

class C
  private_constant :X
  X = ...
end

The current behaviour is to raise NameError.

/tmp/v8svpb4/95:2:in `private_constant': constant C::X1 not defined (NameError)

  private_constant :X1
  ^^^^^^^^^^^^^^^^
	from /tmp/v8svpb4/95:2:in `<class:C>'
	from /tmp/v8svpb4/95:1:in `<main>'

This proposal breaks this backward compatibility.

Also I'm concerned about potential typos. It may be hard to find typos.

class C
  private_constant :BEHAVIOUR
  BEHAVIOR = 123 # Remains public unintentionally
end

Maybe we need some sort of foolproof somewhere in this way.

[znz (Kazuhiro NISHIYAMA)]

const_set is already exist. How about private_const_set?

[Eregon (Benoit Daloze)]

How about this?

class C
  x = ...
  ...
  ...
  X = x
  private_constant :X
end

From a VM POV I really dislike having to remember state for a name before the constant is set, that's really messy and ugly for semantics (e.g., what if the constant is never set, it'll still mess up/slow down constant lookup).

Maybe we should have private_constant/public_constant with no arguments set the constant visibility on the frame, like private/public for methods? I guess private constants are often grouped together as well.

private_const_set sounds OK.

[ujihisa (Tatsuhiro Ujihisa)]

I tried both the private_const_set and the idiom with a local variable versions locally, and found that the private_const_set worked better. The fact that the whole statement begins with "private" really helped.
Didn't like the idea to change the internal mode by calling private_constant by itself, because it consumes more human memory, like private by itself.

The below is my draft to introduce private_const_set. If this is the right direction I can send a pull request on GitHub as well, including public_const_set implementation.

diff --git prelude.rb prelude.rb
index 8fd6e6cb772..d17908758a5 100644
--- prelude.rb
+++ prelude.rb
@@ -29,3 +29,11 @@ def to_set(klass = Set, *args, &block)
     klass.new(self, *args, &block)
   end
 end
+
+class Module
+  def private_const_set(sym_or_str, obj)
+    const_set(sym_or_str, obj)
+    private_constant(sym_or_str)
+    sym_or_str
+  end
+end
diff --git spec/ruby/core/module/private_const_set_spec.rb spec/ruby/core/module/private_const_set_spec.rb
new file mode 100644
index 00000000000..6c8b157e374
--- /dev/null
+++ spec/ruby/core/module/private_const_set_spec.rb
@@ -0,0 +1,26 @@
+
+require_relative '../../spec_helper'
+require_relative '../../fixtures/constants'
+
+describe "Module#private_const_set" do
+  it "sets a private constant specified by a String or Symbol to the given value" do
+    ConstantSpecs.private_const_set(:CS_PCONST401, :const401)
+    -> {
+      ConstantSpecs::CS_PCONST401
+    }.should raise_error(NameError)
+    ConstantSpecs.public_constant(:CS_PCONST401)
+    ConstantSpecs::CS_PCONST401.should == :const401
+
+    ConstantSpecs.private_const_set('CS_PCONST402', :const402)
+    -> {
+      ConstantSpecs::CS_PCONST402
+    }.should raise_error(NameError)
+    ConstantSpecs.public_constant(:CS_PCONST402)
+    ConstantSpecs::CS_PCONST402.should == :const402
+  end
+
+  it "returns the value set" do
+    ConstantSpecs.private_const_set(:CS_PCONST403, :const403).should == :CS_PCONST403
+    ConstantSpecs.private_const_set('CS_PCONST404', :const404).should == 'CS_PCONST404'
+  end
+end

[sawa (Tsuyoshi Sawada)]

Related to #8275, #16276, #16752, #17171.

[okuramasafumi (Masafumi OKURA)]

I wonder if the code below is acceptable:

class C
  private_constant {
    X = ...
  }
end

Here, private_constant takes a block and every constant defined there are private. This is especially useful when defining multiple private constants at once.

[Dan0042 (Daniel DeLorme)]

Another idea:
private_constant X: expr
equivalent to
const_set :X, expr + private_constant :X

Or just use class variables :-)