gistfile1.txt

(19:56:26) #moose-dev: Topic for #moose-dev set by ether!karene@community.opers at 12:44:54 PM on 03/11/2015
(13:01:32) autarch: ether: looking more closely at does_via_delegation, I'm not convinced this implementation is at all sane
(13:01:49) autarch: having the metaclass rely on looking at an instantiated object to decide this seems very, very wrong
(13:02:00) autarch: metaclasses should provide info about the class, not the instance
(13:04:15) autarch: I'm not sure this is actually something we _can_ implement - if the only way to know if something does an interface is to look at the object, then that isn't a question for the metaclass - the implementation should be in Moose::Object alone
(14:03:05) ether: I think we could answer the question on an uninstantiated class if the attribute type is class_of(..)
(14:03:11) ether: er class_type(..)
(14:03:52) ether: I could go both ways on whether it's reasonable for $obj1->DOES(..) to return a different result than $obj2->DOES(..)
(14:04:14) ether: well, we have all month to make it more sane or take it out
(14:04:53) ether: where's stevan, to wax philosophical? :)
(14:13:08) autarch: I really don't like having this in the metaclass as-is - it's just wrong
(14:13:17) autarch: we can
(14:14:06) autarch: we can only answer it in the metaclass if the attribute is required, I think, which makes it really weird - I think if we want this it should just live in Moose::Object
(14:52:07) ether: that sounds sane
(11:26:27) autarch: ether: hmm, the more closely I look at this does-via-delegation the more I dislike it
(11:27:02) autarch: if the object we delegate to is lazy, then the does check continues on - but there's no guarantee that the object the lazy build/default generates will actually implement the role interface!
(11:27:34) autarch: this can only be guaranteed if there is a TC for the attribute that guarantees the role, but currently that's not being checked
(11:27:53) ether: I thought the TC check was one of the preconditions?
(11:28:10) ether: no time to look now - packing up a boat
(11:28:36) autarch: nope, it doesn't check the TC at all, it just looks at the value of $attr->handles
(11:28:48) autarch: I'm inclined to say that this should be removed for now
(11:29:04) ether: if we were to keep it we need more tests I think
(11:29:14) ether: especially more tests of situations where the call returns false
(11:29:52) autarch: agreed
(11:30:04) autarch: I think for now this is a little too half-baked, and we need to send it back to the oven ;)
(12:06:41) mst: autarch: is there a thread/issue/PR/whatever for what you're discussing?
(12:06:58) autarch: mst: it was already merged, but see https://github.com/moose/Moose/pull/104
(12:09:02) mst: ah. this is one of those cases where having attributes and modifiers in roles complicates our lives
(12:09:18) mst: if we only had methods and 'requires' statements then the main class could simply consume the role
(12:10:38) mst: but the implementation is completely inside out
(12:10:56) autarch: only if it consumed the role before the attribute with the delegation was declared, which has its own issues
(12:11:10) mst: uh, no, it would have to consume it afterwards
(12:11:21) autarch: oh right
(12:11:27) mst: so that the handles-installed methods stand as the implementation
(12:12:07) mst: so, for roles that simple, "handles => $role" would simply supply an implicit "with $role" afterwards (not suggesting we do exactly that, talking conceptually)
(12:14:12) autarch: yeah, that would make sense in a different universe ;)
(12:14:27) autarch: a kinder, gentler universe
(12:17:53) mst: right, but basically what we want, I think, is an additional role application type or so, that the attribute application stuff can call
(12:18:13) mst: currently it's possible to enumerate all roles consumed by a class
(12:18:42) mst: does_via_delegation throws that out the window, which makes it clearly the wrong approach
(12:19:28) autarch: I also don't think runtime determination of this stuff _for an object_ is a great idea
(12:19:38) autarch: I guess I'm just repeating what you said with different words
(12:20:36) mst: well, you're identifying pragmatic issues of the implementation whereas I'm identifying the reasons it was conceptually wrong before it even started
(12:21:10) mst: this is not really a bad thing, since a decent patch set should have neither set of problems ;)
(12:21:32) autarch: except that we can't actually apply that additional role because of the issues you brought up
(12:21:59) mst: well, we can't use the normal application code
(12:22:26) mst: but I do wonder whether we -can- mark the role as applied
(12:22:28) autarch: also, I don't think even without those issues we could apply the role - the class only does the delegated role _if_ the attribute is populated, which can't be determined until runtime, unless we restrict this to only required attributes or lazy attrs with the right type constraint
(12:23:25) autarch: there's nothing preventing me from writing this - has foo => ( is => 'ro', handles => 'Some::Role', default => sub { ObjectWithoutSomeRole->new } )
(12:23:41) mst: hrm. I would argue that using "handles => 'RoleName'" is a promise by the class to ensure the thing delegated to can deal with it
(12:24:10) mst: I mean, at that point, $obj->method_from_some_role is going to die
(12:24:17) mst: but, uh, there's no reason you can't write
(12:24:20) mst: with 'Some::Role';
(12:24:27) mst: sub method_from_some_role { die "I LIED!" }
(12:24:30) autarch: we could certainly make it add a TC if you use that form of handled
(12:24:35) autarch: of handles*
(12:24:58) autarch: that would actually make a lot of sense, and isn't really related to this other stuff
(12:25:37) mst: I ... am not immediately convinced that's obviously a net win
(12:26:03) mst: possibly adding the TC if you use that form -and- don't specify a TC would be ok
(12:26:26) mst: but I can imagine e.g. using a wrapper object around a non-Moose object that provides the interface but doesn't answer to does
(12:26:35) mst: where adding the TC would actually break my code
(12:30:19) autarch: yeah, that's a good point
(12:30:56) autarch: honestly, this seems like a lot of work to core a feature of very marginal utility - if people really want this then at the very least I'd like to see th MX module improved to be more robust before we even think about coring this
(12:31:44) mst: hrm. I think the implementation I'm describing could be done as a meta-attribute trait.
(12:31:59) autarch: yeah, for sure
(12:32:06) autarch: but it should live as an MX for a while
(12:32:23) mst: that was kinda my point. a meta-attribute trait would work as an MX.
(12:32:35) mst: also people can opt their attributes in
(12:32:50) autarch: I think we agree
(12:32:52) mst: once that's shaken out, we can consider whether it should be enabled for all handles-ish code