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

Issue 4999 - Add Kenji Hara's adaptTo() to Phobos #1265

Merged
merged 3 commits into from
Jun 2, 2013

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Apr 18, 2013

http://d.puremagic.com/issues/show_bug.cgi?id=4999

Changes:

  • Add structuralCast and structuralDownCast in std.typecons module
  • Remove unused and undocumented structuralCast which was in std.exception.

@ghost
Copy link

ghost commented Apr 19, 2013

Could you indent predicates so they're like this:

template T()
    if (...)
{
}

It makes it easier to distinguish a template/function definition to a regular if statement block.

@DmitryOlshansky
Copy link
Member

I like the functionality but structuralCast ?!
I thought it was all about casting structs having the same field structure and then turns out it's about adapting 2 unrelated clases/interfaces having the same sets of functions. I'd call it duckCast !!!

@9rnsr
Copy link
Contributor Author

9rnsr commented Apr 21, 2013

My attitude for the indentation of template constraint part is not determined. In past days, I used the same style that you suggested (std.conv and std.format is mostly based on the style). But after a while, I noticed that the style has the problem with complex template constraints.

template T()
    if (... && ... &&
        ... && ...)    // 8 indentation
{
}

If the constraint needs more than one line, the second line needs 8 space indentation. I feel it is "too far" from the left edge.
Again, I don't have strong argument for the style. But it is not yet determined in the "Phobos style guide". So I'd like to keep this.

@9rnsr
Copy link
Contributor Author

9rnsr commented Apr 21, 2013

I like the functionality but structuralCast ?!

Originally that was named adaptTo. I renamed it to structuralCast based on the Andrei's suggestion.
Better name is welcome!

((FA0 ^ FA1) & (FA.ref_ | FA.property)) == 0)
{
alias R = Select!(is(R0 : R1), R0, R1);
alias R FX(P0);
Copy link

Choose a reason for hiding this comment

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

What does this code do? It looks really weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes a function type FX that has a return type R and parameter types P0.

Copy link
Member

Choose a reason for hiding this comment

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

C function type syntax. Eww…

Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure that's slated for deprecation, no?

Copy link

Choose a reason for hiding this comment

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

Yeah I think it will be. Seems like alias FX = R function(P0); would do here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In here, FX should be a function type, not function pointer type. So, alias FX = R function(P0); is not correct.

Copy link
Member

Choose a reason for hiding this comment

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

Couldn't you just use FunctionTypeOf!(R function(P0)) or something like that? I think that would make the code much more readable.

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's good idea. Fixed.

@sgraf812
Copy link

Concerning names: How about

Orig obj = ...;
auto ifc = wrapAs!Interface(obj); // or
auto ifc = obj.wrapAs!Interface; 
// and
obj = unwrapTo!Orig(ifc); // or
obj = ifc.unwrapTo!Orig;

and extend it to work on structs? Haven't tried it, just based on what David wrote.
It would merely involve an alias-this in a templated class.

@andralex
Copy link
Member

I think we need short, memorable, single-word names like wrap and unwrap.

@DmitryOlshansky
Copy link
Member

I think we need short, memorable, single-word names like wrap and unwrap.

+1

@9rnsr
Copy link
Contributor Author

9rnsr commented May 27, 2013

Renamed function names to wrap/unwrap.

@@ -2602,6 +2602,623 @@ template generateAssertTrap(C, func.../+[BUG 4217]+/)
}

/**
* Supports structural based typesafe conversion.
Copy link
Member

Choose a reason for hiding this comment

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

Great. We'd need some links and expanded documentation and examples. (One simple reference would be e.g. http://en.wikipedia.org/wiki/Structural_type_system).

andralex added a commit that referenced this pull request Jun 2, 2013
Issue 4999 - Add Kenji Hara's adaptTo() to Phobos
@andralex andralex merged commit cee7de0 into dlang:master Jun 2, 2013
@andralex
Copy link
Member

andralex commented Jun 2, 2013

This will be a very influential piece of work, thanks @9rnsr, There are quite a few ways in which we can expand on this idea, will flesh them out later.

@ghost
Copy link

ghost commented Oct 20, 2013

I'm a little confused about this, since I thought wrap was supposed to be generic and allow code such as:

interface Interface
{
    int foo();
    int bar(int);
}

struct Pluggable  // note that it's a struct
{
    int opDispatch(string name, A...)(A args) { return 100; }
}

Interface i = wrap!Interface(Pluggable());

This currently errors inside of phobos with:

std\typecons.d(2875): Error: template std.typecons.dynamicCast does not match any function template declaration. Candidates are:

If the wrap function is only supposed to be used with objects it should be constrained to disallow non-class types, and it should also be properly documented.

@9rnsr
Copy link
Contributor Author

9rnsr commented Oct 26, 2013

@AndrejMitrovic It's a current implementation limitation which I had not recognized. For unwrap operation, current implementation uses the Objecttype as a most generic base type. However it won't work for any struct types.
We should relax the limitation in the near future release.

@ghost
Copy link

ghost commented Oct 26, 2013

Ok, great to hear.

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

Successfully merging this pull request may close these issues.

7 participants