Feature #3908: private constant

On Tue, Oct 5, 2010 at 17:19, Yusuke Endoh <[e-mail]> wrote:

> What do you think?

Yes!  Finally!  Thanks!

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r29603.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

I'm sorry I missed the deadline on this ticket...

1) Is it intentional that `const_get :K`, where :K is a private constant, raises a NameError? In the context of private methods, the reflection API only enforces visibility constraints if they have been explicitly requested (e.g. `#methods` c.f. `#private_methods`).

2) Should private constants be returned by `#constants`? They currently are, but `#methods` excludes private methods. That is, all methods returned by `#methods` may be objectified with `#method`, yet the same is not true for constants. Further, as there is no predicate for determining whether a constant is public or private, the programmer must resort to rescuing NameError when using `#const_get`. This represents an incompatibility in that existing code of the form `K.constants.map{|c| K.const_get c}`, where `K` contains one or more private constants, will now raise an exception. 

3) Is it intended that, given the name of a private constant, `#set_const` makes it public?

    class K
      C = true
      private_constant :C
    end
   K::C
   #=> NameError: private constant K::C referenced
        from (irb):6
        from /usr/local/bin/irb:12:in `<main>'
   K.const_set :C, false
   (irb):7: warning: already initialized constant C
   #=> false
   K::C #=> false

I found a typo in error message in r29603.

Index: variable.c
===================================================================
--- variable.c	(revision 29612)
+++ variable.c	(working copy)
@@ -1895,7 +1895,7 @@

     if (rb_safe_level() >= 4 && !OBJ_UNTRUSTED(mod)) {
 	rb_raise(rb_eSecurityError,
-		 "Insecure: can't change method visibility");
+		 "Insecure: can't change constant visibility");
     }

     for (i = 0; i < argc; i++) {

Hi,

2010/10/27 Run Paint Run Run <[e-mail]>:
> Issue #3908 has been updated by Run Paint Run Run.
>
>
> I'm sorry I missed the deadline on this ticket...

No problem.  Thank you always for your detailed spec review.
# Say, I wanna work on File.write...


> 1) Is it intentional that `const_get :K`, where :K is a private constant, raises a NameError?

No.  I think `#const_get' does not raise an exception for
private constants because `#method' and `#send' does not
for private methods.

Matz, may I introduce a new method `#public_const_get', like
`#public_send'?


> 2) Should private constants be returned by `#constants`?

I think no.  I'm attaching a patch.

Matz, may I introduce two new methods, `#public_constants'
and `#private_constants'?


> 3) Is it intended that, given the name of a private constant, `#set_const` makes it public?

No.  I'm attaching a patch.


NOTES:

  - Matz is now casting doubt on this feature ;-( because
    we've not estimated the impact of this feature yet.
    Though I thought that I certainly got matz's approval,
    I might make a quick judgment.
    We're discussing still now, but this feature may be
    reverted.

  - The first patch slightly changes YARV insn format for
    defineclass, to distinguish constant access style in
    class definition, e.g.,
      OK: class Foo::PrivateClass
      NG: class PrivateClass
    Thus, ko1's approval is needed.

  - I have to work on defined?(Foo::PrivateConstant)

-- 
Yusuke Endoh <[e-mail]>

Attachment: 0001-add-rb_const_get_-_visi-functions.patch
Attachment: 0002-const_set-should-not-change-constant-visibility.patch
Attachment: 0003-Module-constant-should-exclude-private-constants.patch

• Status changed from Closed to Assigned

Hi,

2010/10/27 Tomoyuki Chikanaga <[e-mail]>:
> I found a typo in error message in r29603.

Thank you always for your detailed code review.
I'll import your patch after matz's decision.

-- 
Yusuke Endoh <[e-mail]>

> Matz is now casting doubt on this feature ;-( because we've not estimated the impact of this feature yet. 
> Though I thought that I certainly got matz's approval, I might make a quick judgment. We're discussing 
> still now, but this feature may be reverted.

It would be a shame if it's reverted. There are a few rough edges, but nothing insurmountable. FWIW, I agree with your changes.

• Status changed from Assigned to Closed

This issue was solved with changeset r30290.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

Hi,

Yusuke, patches in the mail I'm replying are not applied and we still
need discuss about followings, right?

- Module#constants includes a private constant?
- Module#const_get raises NameError for a private constant?
- add Module#public_constants and Module#private_constants?
- Module#const_set changes the constant visibility to public? Setting
visibility via const_set?

Just as a notification since I see this ticket closed today.

Regards,
// NaHi

• Status changed from Closed to Assigned

Hi, NaHi

> Just as a notification since I see this ticket closed today.

Kazu committed a change of NEWS with ML ref, which closed this
ticket.  I think that he did not intend to close this ticket.
That is, it is just accident.  I'm reopening this ticket.


> Yusuke, patches in the mail I'm replying are not applied

Yes, just because of my laziness.  I will work on.


> and we still need discuss about followings, right?

I think that what we needed is not discussion, but matz's final
approval.
The reason why this ticket stopped was because matz cancelled
his approval and opposed this feature at [ruby-dev:42469].
And, matz has finally re-approved it at [ruby-dev:42587].
So we can now move on.


> - Module#constants includes a private constant?

It should not.  I will.


> - Module#const_get raises NameError for a private constant?

It should not.  I will.


> - add Module#public_constants and Module#private_constants?

Plus, Module#public_const_get.  But these may be need matz's
additional approval.


Matz, can I add these methods:

  - Module#public_constants, which returns an array that contains
    only public constant names.

  - Module#private_constants, which returns an array that contatins
    only private constant names.

  - Module#public_const_get, which returns a value assigned to the
    constant if it is public, or raise a NameError if it is private.

?


> - Module#const_set changes the constant visibility to public? Setting
visibility via const_set?

It is just a bug.  I will.

-- 
Yusuke Endoh <[e-mail]>

Hi, matz

May I introduce these methods in relation to private constants?

  - Module#public_constants, as alias to Module#constants
    (corresponding to Ojbect#public_methods)

  - Module#private_constants, which returns an array that contains
    only private constant names.
    (corresponding to Ojbect#private_methods)

  - Module#public_const_get, which return the constant value or
    raise a NameError if the constant is private
    (corresponding to Object#public_send)

  - Module#public_const_defined?, which tells whether the public
    constant is defined or not
    (corresponding to Module#public_method_defined?)

  - Module#private_const_defined?, which tells whether the private
    constant is defined or not
    (corresponding to Module#private_method_defined?)

A patch and tests are attached.  Thank you.



Note: I fixed other bugs which RPRR and nagachika reported (r30713,
r30714, r30715, r30716 and r30718).  Sorry for late action.


2010/12/22 Yusuke Endoh <[e-mail]>:
> Issue #3908 has been updated by Yusuke Endoh.
>
> Status changed from Closed to Assigned
>
> Hi, NaHi
>
>> Just as a notification since I see this ticket closed today.
>
> Kazu committed a change of NEWS with ML ref, which closed this
> ticket.  I think that he did not intend to close this ticket.
> That is, it is just accident.  I'm reopening this ticket.
>
>
>> Yusuke, patches in the mail I'm replying are not applied
>
> Yes, just because of my laziness.  I will work on.
>
>
>> and we still need discuss about followings, right?
>
> I think that what we needed is not discussion, but matz's final
> approval.
> The reason why this ticket stopped was because matz cancelled
> his approval and opposed this feature at [ruby-dev:42469].
> And, matz has finally re-approved it at [ruby-dev:42587].
> So we can now move on.
>
>
>> - Module#constants includes a private constant?
>
> It should not.  I will.
>
>
>> - Module#const_get raises NameError for a private constant?
>
> It should not.  I will.
>
>
>> - add Module#public_constants and Module#private_constants?
>
> Plus, Module#public_const_get.  But these may be need matz's
> additional approval.
>
>
> Matz, can I add these methods:
>
>  - Module#public_constants, which returns an array that contains
>    only public constant names.
>
>  - Module#private_constants, which returns an array that contatins
>    only private constant names.
>
>  - Module#public_const_get, which returns a value assigned to the
>    constant if it is public, or raise a NameError if it is private.
>
> ?
>
>
>> - Module#const_set changes the constant visibility to public? Setting
> visibility via const_set?
>
> It is just a bug.  I will.
>
> --
> Yusuke Endoh <[e-mail]>
> ----------------------------------------
> http://redmine.ruby-lang.org/issues/show/3908
>
> ----------------------------------------
> http://redmine.ruby-lang.org


-- 
Yusuke Endoh <[e-mail]>

Attachment: private-constant-omake.patch

• Assignee changed from Yusuke Endoh to Yukihiro Matsumoto
• Target version changed from 1.9.3 to 2.0.0