Skip to content

Commit

Permalink
Kernel#instance_variables should not include private _klass and _id vars
Browse files Browse the repository at this point in the history
  • Loading branch information
@adambeynon
adambeynon committed Oct 20, 2013
1 parent bd9daa9 commit 53064ef
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
4 changes: 3 additions & 1 deletion corelib/kernel.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,9 @@ def instance_variables
for (var name in #{self}) {
if (name.charAt(0) !== '$') {
result.push('@' + name);
if (name !== '_klass' && name !== '_id') {
result.push('@' + name);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions corelib/module.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ def self.new(&block)
klass._klass = Module;
klass.__dep__ = []
klass.__mod__ = true;
klass._proto = {};
// inherit scope from parent
$opal.create_scope(Module._scope, klass);
Expand Down

3 comments on commit 53064ef

@DouweM
Copy link
Contributor

@DouweM DouweM commented on 53064ef Oct 20, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about _isString and the likes? What about klass._proto etc in Class#instance_variables?

@meh
Copy link
Member

@meh meh commented on 53064ef Oct 20, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a thousand other special attributes? Something tells me we should have an attributes table or something.

@adambeynon
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a very good point. _klass and _id were creating a really hard to track down bug in rspec as objects were having their class changed which mucked up super chains. It is fair to say that a lot of other properties would cause equal damage, e.g. a class having its _alloc or _proto property swapped out for something else.

It is also a concern that these are valid ivar names, so it is possible to access and change these properties from ruby code. Maybe we should consider either an ivar table, as @meh suggests, or perhaps a suffix for opal special variables.

I personally prefer the suffix idea, as I am quite often going through object literals in the chrome debugger and having these special vars only 1 level deep makes it a lot easier to debug problems.

Please sign in to comment.