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

support Type::Tiny #81

Open
dams opened this issue May 31, 2013 · 33 comments
Open

support Type::Tiny #81

dams opened this issue May 31, 2013 · 33 comments
Labels

Comments

@dams
Copy link

dams commented May 31, 2013

It's all in the title... Type::Tiny is great, fast, light, minimalist, and works well with Moo. Can it be supported by M::S ? thanks

@barefootcoder
Copy link
Contributor

Well, trying to find a way to support Moo is definitely on my list. I was looking at MooX::Types::MooseLike (somewhat doubtfully), but perhaps this is another alternative. I won't have any time until after YAPC, but I'll look into this first chance I get.

Thx for the tip!

@schwern
Copy link
Contributor

schwern commented Jun 1, 2013

Once #34 is done, Type::Tiny can definitely be added to the set of introspected type systems. However, allowing lexical scalars as types may cause type signature parsing to become ambiguous. That'll have to be thought through some.

my $NUM = "Type::Tiny"->new(
   name       => "Number",
   constraint => sub { looks_like_number($_) },
   message    => sub { "$_ ain't a number" },
);

method add( $NUM $left, $NUM $right ) {
    ...
}

@dams
Copy link
Author

dams commented Jun 3, 2013

Didn't have more than 2 hours to spend... However, this simple hack makes the standard types supported:
dams@d1eb0e3
As this test demonstrates it:
dams@20a5367
Not sure how you guys want Type::Tiny types to be enabled. An option on M::S import ? or should Type::Tiny be checked before Any::Moose ? Also, I'll try to make class/rolle based types work. About custom types, I think that it's just a matter of the user adding it to the Type::Tiny registry that we would have built for M::S, so an ad-hoc method to do just that should be enough (so no need to parse custom types as scalar in the method signature). Any hint, help or guidance welcome.

@dams
Copy link
Author

dams commented Jun 15, 2013

Please see Pull Request #83. It's probably better to continue discussion over there and close this bug.

@barefootcoder
Copy link
Contributor

Well, I've finally found the time to review Type::Tiny, and I must say: it seems to me like it might be worthwhile to consider just using Type::Tiny for everything. I can't see anything that Moose can do that Type::Tiny can't, including some things that Mouse can't do. It's small, it's fast, it would basically moot #34, it works with Moose, Mouse, and Moo ... where's the downside to switching over to it for everything?

AFA this:

my $NUM = "Type::Tiny"->new(

goes, that's an anonymous type, and I don't see any reason to allow those in method signatures.

@rjattrill
Copy link

We get Types::Standard available by default with this I assume.

On Thu, Jul 4, 2013 at 2:11 PM, Buddy Burden [email protected]:

Well, I've finally found the time to review Type::Tiny, and I must say: it
seems to me like it might be worthwhile to consider just using Type::Tiny
for everything. I can't see anything that Moose can do that Type::Tiny
can't, including some things that Mouse can't do. It's small, it's fast, it
would basically moot #34#34,
it works with Moose, Mouse, and Moo ... where's the downside to
switching over to it for everything?

AFA this:

my $NUM = "Type::Tiny"->new(

goes, that's an anonymous type, and I don't see any reason to allow those
in method signatures.


Reply to this email directly or view it on GitHubhttps://github.com//issues/81#issuecomment-20458294
.

@dams
Copy link
Author

dams commented Jul 4, 2013

@barefootcoder I agree, I also think that using Typ::Tiny everywhere is a great choice.
@rjattrill: in my PR yes you get types::standard by default, and you can also load additional types at "use" time.

@barefootcoder
Copy link
Contributor

We get Types::Standard available by default with this I assume.

Well, my suggestion is that you get it by default, by not-default ... you just get it, period. I may try some benchmarks with the Moose type vs Type::Tiny, because I suspect that Type::Tiny will be faster, but I'll need to prove it. But, assuming Type::Tiny is faster, it appears to be just as featureful, you can create new types with it just as easily, it handles coercions just as well, and it works seamlessly with Moose (as well as all the others). So I'm having difficulty imagining why, as a user, you'd ever not want to use it.

I guess what I'm asking is, can anyone think of any reason why someone would prefer using Moose types over Type::Tiny?

Ah, here's one: what if you already have a bunch of Moose types built up--subtypes and enums and whatnot. You have your own type registry going, and you don't want to switch to Type::Tiny (why not, other than just not having the time to do it, I can't imagine, but that's not for us to say). Can Type::Tiny handle user-defined types, if those types are defined via Moose?

@dams
Copy link
Author

dams commented Jul 4, 2013

In my opinion, M::S should be flexible enough to be used with Moose types or Types::Tiny, and maybe others.

As you said, some people may want to prefer Moose types because they work on existing code that uses it and they cannot migrate, or they use specific features of Moose types (if any).

However, one big advantage of Type::Tiny is the fact that you can have more than one Type::Registry, instead of the global namespace for Moose types, which forces you to heavily namespace your types.

Also, what about backward compatibility ? it would be a shame that a new version of M::S breaks your code and forces you to migrate to Type::Tiny

Last but not least, what if other types implementation emerge ? we've seen Moose, Mouse, then Moo (I won't mention the other Mo* stuff...) And it's difficult to see in advance which one are going to be used ultimately.

So if there can be a clever way to be compatible with any type implementation, I'm all for it.

@barefootcoder
Copy link
Contributor

@dams: Let me take your points one at a time:

... some people may want to prefer Moose types because they work on existing code that uses it ...

Using Moose types for your attributes and Type::Tiny for your method parameter types should present no problems as long there aren't any differences (even subtle ones) between the two implementations for the same type, which I don't believe there are.

... or they use specific features of Moose types (if any).

But that's what I'm saying: I believe that the way @tobyink has designed Type::Tiny is that it both is and always will be feature-compatible with Moose types.

Also, what about backward compatibility ?

Given feature compatibility, I can't see any way that this change would break backcompat.

Last but not least, what if other types implementation emerge ? we've seen Moose, Mouse, then Moo (I won't mention the other Mo* stuff...)

Well, exactly. Type::Tiny is a way to get off this demonic merry-go-round. If we say, "we don't care what crazy system you're using, and whatever types it wants to use, we're going to use Type::Tiny" then we don't give a crap what the next type craze is.

Now, I'm making some assumptions here (faster implementation, full feature compatibility, commitment to maintain such, etc). I'm going to have to do some investigation to verify these things. I could be wrong, and, if I am, then the question is moot and we'll have no other choice than to support multiple implementations. But, if I'm right ...

Why not?

@tobyink
Copy link

tobyink commented Jul 5, 2013

Here's a few random points...

  1. If you've got a MooseX::Types type constraint, you can convert it to Type::Tiny using Types::TypeTiny::to_TypeTiny( SomeMooseXTypesType ).
  2. The above is automatically done by Type::Tiny all over the place. So if ArrayRef is a Type::Tiny type, and Foo is a MooseX::Types type, then ArrayRef[Foo] ought to "just work".
  3. With regards to speed, most of my benchmarking has used type constraints within Moo/Mouse/Moose attributes. Type::Tiny outperforms all of them except Mouse::XS. However I suspect that Method::Signatures doesn't get the full benefit of Mouse::XS's speed anyway, because it interacts with it on the Perl side rather than calling the C functions directly.
  4. Type::Tiny is planned to track Moose/Mouse/Moo's API as closely as possible. Of course it may be that in the future they diverge so far that tracking them all becomes impossible.
  5. "what if other types implementation emerge" - there is Specio which is a project with similar aims to Type::Tiny, but a different (somewhat saner) API. As of current versions, it cannot be dropped into Moo/Mouse/Moose and actually work - because Mouse/Moose want a type constraint object with an insane API - but with patches in future versions of the OO frameworks, they may offer support. (In which case, Type::Tiny's API will change to match Specio's, at least as far as required for point 4 above.)
  6. Converting a string such as "ArrayRef[Int]" into a type constraint object is something that's fairly new in Type::Tiny. This is the Type::Registry stuff in dams' patch. The next version of Type::Tiny will hopefully offer a bit more DWIM here; for example, if you have a string like "Foo::Bar" and Foo::Bar doesn't correspond to a known type constraint, it will assume you meant "InstanceOf['Foo::Bar']".
  7. There are one or two issues with dams' patch. In particular, calling "Type::Registry"->for_me from within Method::Signatures will always return the same Type::Registry object, so you lose the benefit of per-package type registries. You want "Type::Registry"->for_class($class_being_compiled). (Or better, the DWIM stuff in the next release.)
  8. Don't add a dependency on MooX::late. The DWIM stuff in next release is basically a port of the DWIM stuff that's already in MooX::late.
  9. It would be really nice to have a module offering method signatures using Type::Tiny. I'd use it. :-) Function::Parameters kind of supports Type::Tiny, but using a less nice syntax than it does for Moose types.
  10. Any change should probably be preceded by a longish period of trial releases, as this may cause problems for people using Method::Signatures. But if I understand things correctly, with Data::Alias b0rked, you've probably got a longish period of trial releases coming up anyway.

@thoughtstream
Copy link
Contributor

FWIW, I would be delighted if M::S could drop all it's Moose/Mouse
dependencies.

I have nothing against that worthy ecosystem, except that it makes using
M::S a
much heavier proposition and correspondingly less viable in certain types
of
development.

If Type::Tiny can reduce the M::S memory footprint, I'm all for it!

Damian

@barefootcoder
Copy link
Contributor

@tobyink: Thanks for all the detailed explanations. I'm just waiting to hear from @schwern at this point to get his input. I may try to fiddle with the idea on a branch, as a proof of concept.

One question/clarification: You talk about the automatic translation of Moose types to Type::Tiny types. Let's say that a user creates one or more new Moose types, using subtype or enum or what-have-you. Will I be able to write the type checking for MS in such a way that, if these new types are used as parameter types in sigs, they'll just be automagically converted to appropriate Type::Tiny types? 'Cause, if that could work, then I really can't see any reason not to just go with Type::Tiny for everything here.

@thoughtstream:

I have nothing against that worthy ecosystem, except that it makes using M::S a much heavier proposition and correspondingly less viable in certain types of development.

Well, remember that MS doesn't depend on (or drag in) Moose, which is the heavy one. MS will never use Moose unless you use it first. Mouse is a much smaller dependency. Now, that having been said, here's what perlbloat tells me on 5.14.4:

Moose: 30.4Mb
Mouse: 7.7Mb
Type::Tiny: 2.9Mb

So, while Mouse is a hell of a lot better than Moose (and I don't think worthy of being considered a much heavier proposition), Type::Tiny is even better.

I continue to wait for @schwern, but I'm really feeling more and more like Type::Tiny is the way to go here. Can anyone offer any counterindication to my enthusiasm?

@dams
Copy link
Author

dams commented Jul 5, 2013

@barefootcoder as per the @tobyink remarks, I now understand your approach. The major drawback for me was if a user has a ton of Moose types that he couldn't use in Type::Tiny. If that's transparently handled by T::T, then it's all good. And indeed we can consider T::T as the central compat layer for MOP implementations. But now, I'm tempted to try Specio types in Method::Signatures :)

@tobyink

About the Type::Registry issue: in my patch I always use the same type registry because I wanted all custom types to be available accross all code that uses M::S. A little bit like the global Moose type registry.

If I change the patch to do what you suggest, it means that every class using M::S with custom T::T types will have to load these custom types to their registry each time they use Method::Signatures. Am I right ?

Maybe the solution is to ask people to put their custom types in a single registry that they pass to Method::Signature at use time ? like use Method::Signature registry => $reg. Exposing the registry would be nicer than the load_types option I implemented. But then I'm worried about the syntax complexity.

About DWIM stuff: from my tests, I think this already works fine with my patch. If you look at type_check_type_tiny.t line 41 you'll see that I've commented the line just because the error message is different and the test fails. But it properly recognizes Foo::Bar as being InstanceOf['Foo::Bar']. Is it a strange side effect?

Anyway I'll wait for that DWIM release, and in the mean time try to improve my patch.

@barefootcoder: Shall I give it a new try with a new PR removing Mo*se altogether and relying only on TT ?

@tobyink
Copy link

tobyink commented Jul 5, 2013

Let's say that a user creates one or more new Moose types, using subtype or enum or what-have-you. Will I be able to write the type checking for MS in such a way that, if these new types are used as parameter types in sigs, they'll just be automagically converted to appropriate Type::Tiny types?

The following test script passes (except the TODO) in the latest dev release of Type-Tiny...

use strict;
use warnings;
use Test::More 0.96;
use Test::TypeTiny;

use Moose;
use Moose::Util::TypeConstraints qw(:all);
use Type::Utils 0.015 qw(dwim_type);

# Creating a type constraint with Moose
subtype "Two", as "Int", where { $_ eq 2 };

my $two  = dwim_type("Two");
my $twos = dwim_type("ArrayRef[Two]");

isa_ok($two, 'Type::Tiny', '$two');
isa_ok($twos, 'Type::Tiny', '$twos');

should_pass(2, $two);
should_fail(3, $two);
should_pass([2, 2, 2], $twos);
should_fail([2, 3, 2], $twos);

# Creating a type constraint with MooseX::Types
{
    package MyTypes;
    use MooseX::Types -declare => ["Three"];
    use MooseX::Types::Moose "Int";

    subtype Three, as Int, where { $_ eq 3 };
}

# Note that MooseX::Types namespace-prefixes its types.
my $three = dwim_type("MyTypes::Three");
my $threes = dwim_type("ArrayRef[MyTypes::Three]");

TODO: {
    local $TODO = 'this probably needs fixing';
    isa_ok($three, 'Type::Tiny', '$three');
}

isa_ok($threes, 'Type::Tiny', '$threes');

should_pass(3, $three);
should_fail(4, $three);
should_pass([3, 3, 3], $threes);
should_fail([3, 4, 3], $threes);

done_testing;

I wouldn't like to say that everything would be completely transparent. There are always going to be weird edge cases.

@barefootcoder
Copy link
Contributor

@dams:

Shall I give it a new try with a new PR removing Mo*se altogether and relying only on TT ?

Well, I'd definitely like to have something along those lines in a branch. I'm not sure how you could leave the existing pull request intact and create a whole new one though ... maybe your git-fu is stronger than mine though. :-)

@tobyink:

The following test script passes (except the TODO) in the latest dev release of Type-Tiny...

Wow ... impressive. I'm definitely getting excited about the possibilities here. :-)

@dams
Copy link
Author

dams commented Jul 5, 2013

@barefootcoder creating a new PR is easy :) no problem with that. I'll try to mock up something.

@barefootcoder
Copy link
Contributor

creating a new PR is easy :) no problem with that.

Well, you'll have to tell me how to do it sometime. :-) My experience is, once I create a pull request on my fork, every commit I do to that fork from then on goes into the existing pull request. So I'm not sure how to make it so that you have two different PRs for the same repo. But I'm sure it's possible.

@tobyink
Copy link

tobyink commented Jul 7, 2013

My experience is, once I create a pull request on my fork, every commit I do to that fork from then on goes into the existing pull request.

Branches. You can commit different threads of work to different branches, and then create a pull request per branch.

@dams
Copy link
Author

dams commented Jul 7, 2013

@barefootcoder as @tobyink said, I usually don't commit on master on my repo (forked from upstream, that mean in this case, here). I usually create a branch, commit into it, and then create the PR from within it.

@dams
Copy link
Author

dams commented Jul 19, 2013

@barefootcoder @tobyink @schwern : so, took me some time, but I've come up with a clen PR to have M::S rely purely on Type::Tiny. Check it out: PR #85

Let me know if you like it

@barefootcoder
Copy link
Contributor

Branches. You can commit different threads of work to different branches, and then create a pull request per branch.

Ah, yes, that makes sense. Same branch, same pull request; different branches, different pull requests.

so, took me some time, but I've come up with a clen PR to have M::S rely purely on Type::Tiny. Check it out: PR #85

Excellent, @dams. Thx for the quick turnaround. Hopefully I can take a look this weekend.

@schwern
Copy link
Contributor

schwern commented Aug 7, 2014

Sorry for the late commentary. I love the idea of including Type::Tiny, I think it's a great type ecosystem. I have a problem with using only Type::Tiny.

For a given project, I tend to write custom types. If I'm using Mouse or Moose, I'm going to write them using Mouse or Moose. If I use Method::Signatures, I'm going to expect it to use those types. If MS only uses Type::Tiny then I need one type system for my OO stuff and one for my signatures.

I have exactly this problem, in reverse, with Gitpan. Gitpan is using Moo and thus Type::Tiny. Method::Signatures is using Mouse types. I have a bunch of custom types and coercions that I can't use in signatures. Annoying.

This is why I proposed #34 to choose which type system to use on a per class basis. If your class is using Moose, use Moose types when compiling signatures for that class. If it's using Mouse, use Mouse. Otherwise look for a function that returns a Type::Tiny object. If there's no function, use Mouse (because it's cheap and fast and already loaded). Also offer a way to make it explicit with use Method::Signatures types => "Type::Tiny" or something.

Yeah, it's over-complicated, but we're stuck with multiple type systems until the Mo* folks figure it out.

@tobyink
Copy link

tobyink commented Aug 11, 2014

For what it's worth, Type::Utils (bundled with Type::Tiny) provides a function dwim_type which can be used like this:

dwim_type("ArrayRef[Foo]", for => "Local::Class");

This does a DWIM type lookup from the perspective of Local::Class, using this technique:

  1. It first looks up types that Local::Class has set up in a Type::Registry-based type registry. Type registries are currently a little-used feature, but as of the next release of Type::Tiny they'll be integrated into Type::Library, so any type constraints that Local::Class imported will end up in Local::Class' registry.
  2. Otherwise if a type with that name exists in Types::Standard, it will be used.
  3. Otherwise if Moose is in memory, look it up as a Moose type constraint.
  4. Otherwise if Mouse is in memory, look it up as a Mouse type constraint.
  5. If the type constraint name is unrecognised by all of the above, and resembles a package name, assume a class_type.

It could probably be tweaked. e.g. or the Moose/Mouse lookups, it might be more sensible instead of checking that they are loaded, to check if Local::Class is built using them.

@schwern
Copy link
Contributor

schwern commented Aug 12, 2014

@tobyink It would make me deliriously happy to offload type guessing to some other module. Well volunteered.

The order I would suggest is...

  1. If there is an explicit, lexical type system declaration, use that.
    use Method::Signatures { types => "Moose" }
  2. If $class isa Moose, use Moose
  3. If $class isa Mouse, use Mouse
  4. Do whatever Type::Tiny guessing you feel is best.
  5. Assume a class_type.

@tobyink
Copy link

tobyink commented Aug 12, 2014

I just got a relevant test case passing last night. It defines a Moose class and a Mouse class. It also defines a Moose type constraint called FortyFive and a Mouse type constraint called FortyFive which have conflicting definitions. dwim_type() will provide the correct definition of FortyFive depending on which class the type constraint is requested for.

The one edge case where it has to make a judgement call is: Moose and Mouse are both in memory, and each define a type constraint with the desired name, and the class which is requesting a type constraint is neither a Moose nor a Mouse class. In this case, it prefers the Moose type constraint.

@schwern
Copy link
Contributor

schwern commented Aug 12, 2014

@tobyink Great work!

As to the selection in the ambiguous case, there are problems with the "use Moose if it's in memory" plan. First, it assumes that Moose and Mouse types act equivalently, which is true of the core types but falls on its face for any custom types.

Second, and far worse, is you can't know which it is going to pick. Since it's based on the internal state of the entire process, anything loading Moose might cause your class to start using Moose types. Similarly, if your non-affiliated class is using Mouse types today, something in your dependency chain might use Moose tomorrow. With no warning your class has switched to Moose, your custom Mouse types no longer work, what a PITA to debug.

These are all hateful things about Any::Moose, which I think we can call agree sucks on rice, and it would be silly to keep it in the plan.

There is backwards compatibility to consider. I'm going to split backwards compatibility into two cases. The first is an unaffiliated package using standard Moose types. Since Moose and Mouse have the same standard types, this works fine and should continue to work fine.

The second is a package using custom Moose or Mouse types. This is already unreliable. I'm ok with breaking it if it means putting in a new system that is sensible. For the record, I use custom Mouse types with MS all the time.

I propose that the fallback case is simply Mouse. Why? It's deterministic, there's no action at a distance. Why Mouse and not Moose? MS already loads Mouse and Moose is a pig. It is backwards compatible for the "standard types" case.

An alternative is to rewrite Method::Signatures using Moo and have Type::Tiny be the fallback. This is advantageous if Method::Signatures has to load Type::Tiny just to resolve any types. Since Types::Standard will be loaded by Method::Signatures it can be made backwards compatible for the "standard types" case. And, if @tobyink is going to stick around we can take full advantage of his knowledge of types.

The important thing is that the resolution of the ambiguous type case be made unambiguous and lexically predictable. No action at a distance. No changes in your dependency chain affecting the choice of type system.

We can also add some documentation about how to make types unambiguous throughout your project, because not everything is a class.

package MyProject::Signatures;
use Method::Signatures ();

sub import {
    my $caller = caller;
    return Method::Signatures->import({ into => $caller, types => "Moose" });
}

BTW Currently the biggest, fattest drag on Method::Signatures is PPI. If you want Method::Signatures to load itself and compile signatures faster, that is the target.

@tobyink
Copy link

tobyink commented Aug 18, 2014

Just to be clear, the ambiguous case we're talking about is where Local::Class contains a method signature with a type constraint "Foo", and:

  • Foo is not defined in Types::Standard; and
  • Foo is not defined in a third-party Type::Tiny type constraint library that Local::Class imported; and
  • Moose is %INC; and
  • Local::Class does not use Moose; and
  • Mouse is in %INC; and
  • Local::Class does not use Mouse; and
  • Both Moose and Mouse define a type constraint called Foo with conflicting definitions.

Writing all this out in long hand has led me to the perfect solution (hurrah!)... in this situation it should accept any value that would be accepted by either Moose or Mouse. That is, a union type constraint.

Of course there is still the possibility of load-order-related weirdness. What happens if Moose isn't in %INC, but gets loaded later? At some point you just have to say "you can do this, but if it breaks the only guarantee is we'll let you keep both halves".

@schwern
Copy link
Contributor

schwern commented Aug 18, 2014

Good idea to clarify, because my assumptions for the ambiguous case turn out to be broader. Let me lay out my conditions for the ambiguous case, which are just the inverse of how to figure out the type system.

  • Foo does not explicitly declare its type system.

This would be done in the import call by the author.

  • Foo does not use Moose, Mouse, Moo, Moose::Role, Mouse::Role, Moo::Role, Role::Tiny... nothing to indicate which type system it prefers.

This is most likely to arise in a procedural library, though plenty of people will want to write OO by hand. If the module uses any of these we can make an educated guess as to their preferred type system.

  • Foo does not have an imported type which matches.

Like if Types::Standard is imported.

And that's about it.

It is not conditional on what is in %INC, and it is not conditional on whether Moose or Mouse define a type constraint. The reason? Action at a distance. Some other module loading Mouse or Moose or whatever should not affect the choice of types in an unrelated module, that is the Any::Moose problem.

Using the union of matching type constraints is very clever... and it only adds to the ambiguity. Not only has Method::Signatures silently chosen a type system for you, but now it's silently mashed two types together. What a nightmare to debug!

It gets worse. Method::Signatures does not yet support coercion, but it most certainly will as soon as I have time. What if both types have coercions, which do you choose?

It gets worse. What if someone writes type-ambiguous code, and their call stack does not load Moose but something loads Mouse. Ok, they get Mouse types. A few months later, their code mysteriously breaks. Why? After a lot of frustrating debugging, including diving into the guts of Method::Signatures (or hopefully turning on helpful debugging messages) they find out their code is now using Moose types (or a union of a Moose and Mouse types). Why? Because some module in their call stack started using Moose in an upgrade. Or maybe one stops using Mouse, that will change everything again. This sort of situation happened to Gitpan recently when Net::Github::V3 switched to Moo and all my roles and subclasses broke.

The more I use Types::Standard the more I'm appreciating it's local type constraints. And the more I'm finding Moose and Mouse's global type constraints problematic for the same reasons anything global is problematic.

When it comes to guessing the author's intent and you have no information to go on, don't guess. Provide a usable and unambiguous default. Simplest, and lightest, solution is to use whatever Method::Signatures is already using, and to explicitly define it in the docs in case we change ourselves internally.

@tobyink
Copy link

tobyink commented Aug 18, 2014

Hmmm... I think you've convinced me that dwim_type ought to not fall back to using Moose/Mouse types unless one of the following is true:

  • the caller class/role uses Moose/Mouse; or
  • a fallback to Moose/Mouse is explicitly requested.

In pretty much all the tests for dwim_type the caller explicitly uses either Moose or Mouse, so I think I should be able to make that change without breaking the current Type::Tiny test suite. If so, I'll make the change shortly. If it breaks any of the current tests it will take a while longer. (I've made a commitment to not break anything in the current test suite without giving 6 months' notice.)

@schwern
Copy link
Contributor

schwern commented Aug 19, 2014

On 8/18/14 12:35 PM, Toby Inkster wrote:

Hmmm... I think you've convinced me that |dwim_type| ought to not fall
back to using Moose/Mouse types unless one of the following is true:

  • the caller class/role uses Moose/Mouse; or
  • a fallback to Moose/Mouse is explicitly requested.

Huzzah!

In pretty much all the tests for |dwim_type| the caller explicitly uses
either Moose or Mouse, so I think I should be able to make that change
without breaking the current Type::Tiny test suite. If so, I'll make the
change shortly. If it breaks any of the current tests it will take a
while longer. (I've made a commitment to not break anything in the
current test suite without giving 6 months' notice.)

There's a bunch of options if compatibility is a concern. For one,
%options! dwim_type() takes %options. Add check_if_loaded and default
it to [qw(Moose Mouse)]. Then Method::Signatures can pass in an empty list.

@barefootcoder
Copy link
Contributor

I'm very pleased to see some movement here. I've really been meaning to dive into this for many months now, but several things have conspired to keep me away. Some of that is RL beating me over the head, but a healthy chunk of it is just that thinking about how to test all this stuff makes my brain hurt.

I think the first thing we need is some serious test coverage to make sure that the right types always get loaded. If we could achieve that, we could reach a confidence that using only Types::Tiny actually could/would work. I think it could, and I think @tobyink is working hard to make sure it does. And I'm willing to take the plunge and try it out, if we have some solid test cases to demonstrate that it really does work and catch any breakage.

@schwern, it sounds like you may have some code that you could turn into test cases, or even code that you could send me and I could turn it into test cases. I'd just like to get some concrete code in front of me that demonstrates the potential problems; I think once we get out of the realm of the abstract I can move forward with extending the cases and that sort of thing. I just seem to have some sort of mental block about how to start this whole process.

I'm also still trying to wrap my brain around where $class is going to come from. I guess some code along the lines of "ref @{[$signature->invocant]} || @{[$signature->invocant]}" ... that gets the right answer regardless of whether invocant is $self or $class. But then what about when there is no invocant? People can use types in func signatures too ... right?

Would love to get moving on this so mst and ether can stop stalking me on the internets. :-)

@schwern
Copy link
Contributor

schwern commented Aug 20, 2014

Yup, I can write up some test cases. I have a clear idea how to do this.

$class we can get from caller() by looking up the stack at compile time. I worked this out while trying to fix defaults, Method::Signatures::Signature needs the line number the signature started on.

  my $level = 0;
  while( my @caller = caller($level) ) {
      last unless @caller;

      $level++;

      next if $caller[0] =~ /^(Devel::Declare|Method::Signatures)/;

      return $caller[2];
  }

  croak "Couldn't determine what line this signature is on.";

I'm going to be totally offline for over two weeks. I'll come back to this then.

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

No branches or pull requests

6 participants