-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ->methods to work with constants (and perl 5.28) #299
base: master
Are you sure you want to change the base?
Conversation
Constants created by ‘use constant’ are store in the stash as scalar references, whereas constants created by sub foo () { 42 } are stored as subs-in-globs. Before this commit, $class->mc->methods would return the latter kind of constant, and skip the former, unless someone at some point hap- pened to take a reference to one of the former before ->mc->methods got called. If we are going to be poking around in stashes like this, then we do need to be are of what kind of stuff perl sneaks in there. Constants should not be treated inconsistently because of the internal rep- resentation. Also, if I get my way, most subs will start to be stored in stashes as simple coderefs (which saves LOTS of memory), in perl 5.28. This commit kills two stones with one bird: it handles constants cor- rectly (an existing bug) and gets ready for the optimization that I am almost certain will make it into perl 5.28. We don’t need new tests for the latter, because my optimization makes existing tests fail. (For more info on the optimization, see <https://rt.perl.org/Ticket/Display.html?id=132252#txn-1500037> and also <https://rt.perl.org/Ticket/Display.html?id=129916>.)
Do constants count as methods? I said in #299 that If we can consistently differentiate between constants and methods, I'd be inclined to not have constants in I'm inclined towards this because it solves a trickier problem than just listing all possible things which could be called as a method. I'm inclined to say that if it has a prototype, it isn't a method. But putting prototypes on methods is a common mistake. So I'm inclined to say that if it has a What do you think? |
It sounds good, but I am not sure what you mean by this:
Do you mean to see whether a particular subroutine has been inlined as a constant somewhere? BTW, perl5::i defines a method like this:
which I thought was rather neat. |
On second thought, We can use B to inspect a sub to see whether it is constant. (Is that want you meant be constant-folded?) Is loading B acceptable? Some people complain that it is too bloated. I would write a tiny sub-inspection XS module. |
Yes. Though I've used constants myself as class methods so... we're back to the dilemma I proposed about AUTOLOAD. I'm thinking the idea of
B is already loaded by some compile-time dependency, so that's ok. And it's also used by |
In 5.28, sub b(){2} now creates a shorthand constant the way ‘use constant’ has done since 5.10. So test another, reified constant sub as well.
This is just a proof of concept, with one method showing how this could be implemented.
I have updated the pull request with a patch that gives a proof of concept of how ->mc->methods could work, if you want it to return objects that give info about the methods. I do not intend to do any more work on this, but I have provided a basis that you can take any direction you like. |
I like where this is going. The solution of making MethodInfo delegate to CODE is good to resolve the issue that the same code ref might have different names. A few notes. There's an old open issue about this, #80. And an old PR, #246. It hasn't been re-examined in a number of years, but it's worth perusing. This will apply to more than methods, so We can use perl5i features in perl5i code, as long as it doesn't get circular. For example lib/perl5i/2.pm makes use of |
perl5i’s $class->mc->methods method treats constants inconsistently, because constants created by ‘use constant’ are stored as simple scalar refs, whereas constants created via ‘sub foo(){1}’ are stored as subs in glob.
Also, perl5i fails with an optimization that I hope to include in bleadperl soon, more info on which can be found here: https://rt.perl.org/Ticket/Display.html?id=132252#txn-1500037
The cause of both is the same: perl5i assumes that all subs in stashes reside in globs.