Skip to content
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

Remove bogus code in Moose::Meta::TypeConstraint::equals #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mephinet
Copy link
Contributor

While profiling some hot code I reviewed Moose::Meta::TypeConstraint::equals, and think that the lower half of it is bogus and can be removed:
After the single "return 1" of the method, all following code will return undef and does not have any other side effects as far as I can tell - so I think it can be safely dropped.
All unittests are happy - but please double-check, as I'm by no means an export on the Moose meta classes.

@karenetheridge
Copy link
Member

I only have time for a one-minute drive-by presently, but it looks like most of the relevant history for this sub is in dabed76, which had additional code after all the return undef stuff. Perhaps something important got lost along the way that we should bring back (along with tests that exercises that usecase).

@karenetheridge
Copy link
Member

PS. we might want to add an overload from == to this equals method here. strange that it was omitted.

@autarch
Copy link
Member

autarch commented Nov 25, 2017

Yeah, I was wondering if this might be a bug rather than dead code.

@jozef
Copy link

jozef commented Jan 7, 2020

Long before equals() was also checking parents and there was "return 1" at the end after all the current negative returns. It got removed in 05a5763 which made all of those a waste of cpu cycles. If anyone wants to consider parents, should use is_a_type_of(), or?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants