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

extern(delegate) #61

Closed
wants to merge 0 commits into from
Closed

extern(delegate) #61

wants to merge 0 commits into from

Conversation

marler8997
Copy link
Contributor

No description provided.

@mdparker
Copy link
Member

Thanks for the submission, Jonathan. I'll give it a detailed look in a day or two.

@marler8997
Copy link
Contributor Author

I was messing around with delegates this morning and realized that functions are not ABI-compatible with a delegates that have the same parameters list without the first parameter, i.e.

// These are not ABI-compatible
void foo(Something s, <arguments>) 
void delegate(<arguments>)

I thought these were ABI-compatible, at least when Something was the same width/alignment as void*, but it appears they are not. Basile commented on the forums that adding "extern (C)" to the function works, which if true in all cases means that the problem in the ABI is how the parameters are ordered.

With this new information, I'm going to update the DIP to include an additional semantic feature to define functions that are ABI-compatible with member functions. Will push the new DIP soon.

@marler8997 marler8997 changed the title UFCS Delegates Extension Methods and UFCS Delegates Apr 24, 2017
@marler8997 marler8997 changed the title Extension Methods and UFCS Delegates Delegateable Functions Apr 24, 2017
@UplinkCoder
Copy link
Member

@marler8997 ABI compatibility is not and cannot be guaranteed.

@marler8997
Copy link
Contributor Author

@UplinkCoder correct, you currently cannot define a function that guarantees ABI compatibility with a delegate. This DIP proposes adding that functionality.

@UplinkCoder
Copy link
Member

UplinkCoder commented Apr 24, 2017

@marler8997 my point is that you cannot guarantee ABI compatibility for delegates and member functions in principle.
If you do fixing delegates to allow a this-ptr and a context to co-exist becomes a non-option.

Please address this in your dip.

@marler8997
Copy link
Contributor Author

@marler8997 my point is that you cannot guarantee ABI compatibility for delegates and member functions in principle.

I wasn't aware that all member functions weren't guaranteed to be ABI-compatible with delegates. Could you explain more? Maybe an example or two with an explanation?

If you do fixing delegates to allow a this-ptr and a context to co-exist becomes a non-option.

Please address this in your dip.

Sorry I'm not quite sure what you're saying here. My guess is you're trying to say that modifying member functions to always be ABI compatible with delegates breaks something....?

@ghost
Copy link

ghost commented Apr 25, 2017

Two questions to address, both related to classes:

  • Are the delegatables allowed when the "this parameter" is a final class ?
  • Do the delegatables see the protected member ?

@marler8997
Copy link
Contributor Author

Are the delegatables allowed when the "this parameter" is a final class ?

Short answer, yes, the "this parameter" can be any type so long as it can fit inside a "void*". At first when I came up with delegateable functions I limited myself by taking what I knew from C# extension methods. The first type of an extension method is always a class/struct type. It's true that in most cases this is probably what delegateables would be used for, however, there's no reason that a delegate can't put any type in the context pointer., i.e.

// Note: size_t by definition is always the same size as void*, so it can be a "this parameter" of a delegateable
void contextAddr(size_t this, int x)
{
}
void delegate(int) dg = &10.contextAddr;

This example is a bit contrived, but demonstrates the flexibility.

Do the delegatables see the protected member ?

An interesting question. I thought about this when I was thinking about "extension methods" and when I switched to "delegateable functions" it made the question less relevant. I would say no because a"delegateable function" is more of a low level construct that allows you to create an function that is "ABI-compatible" with a delegate. It's that way on purpose so you don't have to address higher level decisions like you've brought up. Delegateable functions could be used as a building block to define higher level constructs like "extension methods", and that's where you would have to answer those questions like, "should an extension method see private/protected members?" or "should an extension method import the member symbol table?". However, I've purposely designed this feature to be on a lower level to avoid decisions like that, while also being generally useful enough to be used as a building block to other features later on.

@ghost
Copy link

ghost commented Apr 26, 2017

That's in the DIP that you must address the issues. I'm happy to know more about the ideal semantic but this should go in the document, maybe in a section can do cant do or whatever.

@marler8997
Copy link
Contributor Author

@bbasile I think discussing more features would warrant a new and separate DIP. This one is only addressing the creation of functions that are ABI-compatible with delegates. The relationship of a delegateable function to a member function of the "this parameter type" is outside the scope of this proposal.

@marler8997
Copy link
Contributor Author

@UplinkCoder After doing some more research on this proposal I think I figured out what you were trying to say. A nested function cannot have both a "this" parameter and a "stack context" pointer, so yes this would be disallowed, and is something that should be addressed in the DIP. Will update.

@marler8997
Copy link
Contributor Author

@mdparker Just checking in. I'm not sure what my next action item is, am I currently waiting for you to review this PR to merge? Is there an amount of time I should wait before asking what's going on? Any clarification on the next steps and roles/responsibilities for getting this DIP merged would be helpful, thanks.

@mdparker
Copy link
Member

I want to get the backlog cleared up a bit before I look at moving this one forward, so I've not done anything more than skim it yet. I expect I'll get the ball rolling on it in a week or two. In the meantime, if you want more people to help you make sure the PR is in shape, feel free to ask for reviewers on the forums.

@mdparker
Copy link
Member

mdparker commented May 15, 2017

Actually, never mind. I posted an announcement inviting people to review the PR.

My first point of feedback is the spelling of the title. I believe it should be 'Delegatable' rather than 'Delegateable'. I'll dig into it more when I can make time for it.

@jacob-carlborg
Copy link
Contributor

The following cannot be made safe/trusted?

void foo(int a)
{
    assert(a == 5);
}

void main()
{
    int b = 5;
    void delegate() a;
    a.funcptr = cast(void function()) &foo;
    a.ptr = cast(void*) b;

    a();
}

@marler8997
Copy link
Contributor Author

marler8997 commented May 15, 2017

@jacob-carlborg Your example may or may not work depending on the target platform's ABI for delegates. If the foo function happens to pass argument "a" in the same way that a delegate passes the context pointer then it will work, but this is not guaranteed by the language. This DIP provides the semantics for you to define a function that is explicitly ABI compatible with a delegate, The proposed syntax to declare this is to name the first paramter this, i.e.

// this function is ABI compatible with a delegate since the first parameter is named "this"
void foo(int this)
{
    assert(this == 5);
}

The second part of this DIP adds semantics to retrieve a delegate to a "delegateable function" complete with a function pointer and a context pointer. Here's the rest of your example rewritten with the proposed feature:

void main()
{
    int b = 5;
    void delegate() a = &b.foo;
    a();
}

Some nice things to note about the second part:

  • No new syntax needed to retrieve a delegate to a "delegateable function" since it uses existing UFCS syntax
  • We are "piggy backing" off the UFCS semantics so there's no need to add new "lookup semantics" for delegateable functions
  • The semantics are type safe, you'll notice there was no need for any casting.

@jacob-carlborg
Copy link
Contributor

Your example may or may not work depending on the target platform's ABI for delegates. If the foo function happens to pass argument "a" in the same way that a delegate passes the context pointer then it will work, but this is not guaranteed by the language

Aha, I see.

@dushibaiyu
Copy link

It is Great! it just like a little bind!


## Abstract

UFCS allows functions to be called like member functions, however, they are limited in that they cannot be converted to delegates. This is because in most cases they are not ABI-compatible. It is therefore proposed to add sematics to define functions that are ABI-compatible with delegates, called "delegateable functions" and utilize UFCS to retreive delegates for these functions with an object of compatible type.
Copy link

@belka-ew belka-ew May 15, 2017

Choose a reason for hiding this comment

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

It is therefore proposed to add sematics to define functions

"n" is missing in "semantics".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the review

@marler8997
Copy link
Contributor Author

@mdparker Delegateable vs Delegatable. I remember not being sure which version to use. I just googled it and it looks like the actual correct term would be delegable (capable of being "delegated").

Here's how I would summarize the pros/cons for each

feature delegateable delegatable delegable
standard english NO NO YES
convention Keeping the 'e' is an older english convention Removing the 'e' is a modern english convention Not sure this one is a convention, more of a special case, but it's the actual correct one according to webster's dictionary
character count longest not longest or shortest shortest
has the root word delegate in the "name" YES NO NO

I'm at a loss as to which one is the best. Even though "delegable" is the correct one it seems like a special case that differs from other conventions in english (surprise surprise) so most people won't know what it means until they look it up. This decision is a perfect candidate for endless "bike shedding". I'm personally OK with all 3 so I'll change it to whatever the "decision makers" like the most.

@JohanEngelen
Copy link

JohanEngelen commented May 15, 2017

Two comments I have after a few minutes read:

  1. Will the parameter be T this or ref T this? A discussion of with/without ref and an explicit choice for one of the two would be nice, or if both are allowed, what the exact meaning is.
    What happens in the following code?
int f(int this) { return this + 42; }
int f(ref int this) { return this + 1; }
int g() {
    int i = 10;
    auto a = &10.f;
    auto b = &i.f;
    return a() + b();
}

int h(int this) { return ++this + 1; }
int p() {
...
    int i;
    auto b = &i.h; // is this allowed?
...
}
  1. What I find missing is a discussion of the current meaning of the pattern &a.foo, it is only very briefly mentioned. It is currently left as an exercise to the reader to determine how much this complicates the reading of D code (if any). Discussions of examples like this would be great:
// Example of how to pass `&a` as argument using UFCS
void foo(int *);
void g() {
    int a;
    foo(&a);
    (&a).foo();
}
// Example of current meaning of `&a.foo`
struct A { void foo(); }
void g() {
    A sa;
    auto b = &sa.foo; // type of b is `void delegate()`
}

Perhaps a clarification of operator precedence (&a.b is read as &(a.b)).

@marler8997
Copy link
Contributor Author

@JohanEngelen Thanks for the review and the excellent questions.

  1. Will the parameter be T this or ref T this? A discussion of with/without ref and an explicit choice for one of the two would be nice, or if both are allowed, what the exact meaning is.

Both are allowed. When the first parameter of the function is named this it does not modify the type in any way. An int would just be an int and a ref int is still just a ref int. Though it does not modify the type, it does modify the ABI used to pass in the argument, namely, the same way a context pointer is passed in. Also note that even though it's passed in the same way a context pointer would it does not mean it has to be a pointer or a reference type, it can still be whatever type you want so long as the bits can fit inside a void* pointer (this is addressed in the DIP). When it cannot fit, then a compile-time error is asserted.

What happens in the following code?

int f(int this) { return this + 42; }
int f(ref int this) { return this + 1; }
int g() {
    int i = 10;
    auto a = &10.f;
    auto b = &i.f;
    return a() + b();
}

int h(int this) { return ++this + 1; }
int p() {
...
    int i;
    auto b = &i.h; // is this allowed?
...
}

In function g(), the call a() will return 52, and b() will return 11 and both are certainly allowed. The more interesting function p() is also allowed so long as an int can fit inside a void* on the target system. Applications would probably want to use size_t in most cases. Note that since function h accepts the integer by value, the variable i will remain unmodified even though the argument was incremented inside function h.

What I find missing is a discussion of the current meaning of the pattern &a.foo, it is only very briefly mentioned. It is currently left as an exercise to the reader to determine how much this complicates the reading of D code (if any). Discussions of examples like this would be great:

...Perhaps a clarification of operator precedence (&a.b is read as &(a.b)).

I attempted to see what DMD compiler would do with this and everything I tried showed that this currently has no meaning. It's a perfect "hole" waiting to be filled with semantic meaning. The goal of this DIP is to make the syntax/semantics of getting a delegate from a member function the same as getting one from a delegateable function. So your question about operator precedence should be the same for both member functions and delegateable function. I'll have to see where in the DIP I can make this more clear.

@JohanEngelen
Copy link

@marler8997 I'm assuming you are going to spell all that out in the DIP? (I am trying to move you towards tripling (really) the size of the DIP to make everything crystal clear to be included in the language spec ;-))

The code string "&a.b" already has a meaning as you say: "getting a delegate from a member function". What I meant to say is that I think you should mention that, and then point out the similarities and then argue that the new meaning of "&a.b" in contexts where "a" is an int (for example) is along the lines of the present meaning of it in other context.

Also, it would have helped me a lot if there would be a long motivation section at the start of the DIP. From what I've been able to piece together now, the motivation is to be able to create (a subset of simple) functors and pass them to functions using the language-builtin delegate functionality. That's where the part of "when the type fits into a void*" comes from (right?); it feels like a hack.

Another thing to elaborate on: how is this handled:

struct S {
  void foo(int);
}
foo(ref S this, int);
void main() {
   S s;
   auto a = &s.foo;
   a(1);
}

I did not spend a lot of time on this DIP (less than an hour), so my apologies if I'm asking obvious things. Hope my comments help you a little. The DIP seems underspecced at the moment; detailing things out more thoroughly would really help!

@skl131313
Copy link

skl131313 commented May 16, 2017

There's quite a few problems with this. It is really unsafe to use, it is pretty much your responsibility to make sure you are doing it correctly. The way it gets activated is also extremely sketchy, I would not suspect the name of a parameter to change the call convention of a function. Then ultimately what you are proposing is allowing you to use different calling conventions on functions, so why not just propose that instead of having this weird way of activating it. It would make more sense to implement it as an attribute where you know what it is doing, changing the calling convention of the function. Not this sort of hidden meaning.

MSVC does this through attributes as well:

void __stdcall foo()
{
}

Then it could also be used in function pointer types, similarly to how nogc/nothrow is used:

@stdcall void foo()
{
}

void function() funcptr = &foo; // compiler error as it is a different calling convention
                                // and not convertible
alias FuncPtr = void function() @stdcall;
FuncPtr funcptr2 = &foo; // ok

Where as in your implementation it would be another error point, as now it can be used as a delegate it can now no longer be used as a regular function pointer.

@marler8997
Copy link
Contributor Author

There's quite a few problems with this. It is really unsafe to use, it is pretty much your responsibility to make sure you are doing it correctly.

The compiler will produce compile time errors in all the cases I can think of. Can you provide an example where it wouldn't?

The way it gets activated is also extremely sketchy, I would not suspect the name of a parameter to change the call convention of a function.

Currently the calling convention of a function can be changed if one of the following is changed:

  1. the function is moved to a different location (i.e. inside vs outside a struct/class)
  2. parameters are modified
  3. the extern property is modified

This proposal uses convention 2. Modifying the name of the first parameter changes how it is passed into the function, causing it to match the equivalent delegate ABI. You shouldn't be surprised that modifying parameters changes the ABI of the function, this is how every functional language already works.

I will give you that this is a new "way" to modify a parameter that doesn't exist today. Another way you could do this is to use convention 3 (modifying the extern property), i.e.

extern(delegate) void foo(ref File file)
{
    file.writeln("Hello, World");
}
void delegate() dg = &stdout.foo;
dg(); // prints Hello, World

I have no problem with this syntax or the proposed syntax. This technique makes it more clear that the function is using the delegate ABI intead of the standard function ABI. The only disadvantage is refactorability, if you decide to make your delegateable function a member function then you have to rename all references to the first parameter to this or just remove the name since this is implied in member functions. I'm honestly fine with either mechanism, I just want the semantics.

Then ultimately what you are proposing is allowing you to use different calling conventions on functions, so why not just propose that instead of having this weird way of activating it. It would make more sense to implement it as an attribute where you know what it is doing, changing the calling convention of the function. Not this sort of hidden meaning.

You don't want EVERY function to use the delegate ABI so you need some way of activating it. You don't like the way I've proposed which is fine, I don't mind using the extern(delegate) one either. I'll let the decision makers determine which one they like better.

Where as in your implementation it would be another error point, as now it can be used as a delegate it can now no longer be used as a regular function pointer.

Correct, and the DIP already explictly mentions this:

...It follows that a delegateable function is not ABI-compatible with a regular function, so taking the address of one MUST NOT return a function pointer. ...

@skl131313
Copy link

You shouldn't be surprised that modifying parameters changes the ABI of the function, this is how every functional language already works.

You aren't modifying the parameters, you are modifying the NAME of a parameter. Which has no effect, and never has with any functional language i know of, on the ABI.

I can't read everything right now, will read the rest later.

@marler8997
Copy link
Contributor Author

You aren't modifying the parameters, you are modifying the NAME of a parameter. Which has no effect, and never has with any functional language i know of, on the ABI.

The other way to look at is that the parameter doesn't really have a name. You are inserting the keyword this into the spot where the name normally goes, and then omitting the name altogether. Then the context pointer is accessed via this like it would be in a member function, hence providing better refactoribility if you ever want to make it a member function.

I don't see either mechanism as much better or much worse. They both have slight advantages and disadvantages. Using something like extern(delegate) or @delegable is less of a change and probably easier to implement so that's a plus in that column.

@skl131313
Copy link

The other way to look at is that the parameter doesn't really have a name.

I mean how the programmer would perceive it, you never have to do anything special with the parameter name. And then name you put is just an alias for the parameter. It adds complexity to know that it means something different, and it's not something that is very common. This is the first I've seen of it.

What if you want a member function to to be usable as a function pointer? I could see potential for wanting that as well. Your implementation doesn't cover that and you would need another new set of rules if you wanted that, adding more complexity.

I guess there aren't as many failure points as I thought in your implementation. I still prefer the implementation of just giving control of calling convention, it allows you to do more and makes more sense.

@zachthemystic
Copy link
Contributor

Also, it would have helped me a lot if there would be a long motivation section at the start of the DIP.

Adding to what JohanEngelen said, a good order of presentation for the DIP might be:

Section: Rationale (i.e. State the Problem)

  1. What you want to do/be able to do, and why. Short code example (optional).
  2. What you currently must do to achieve the effect in question. Code example. (The code from the DummyType example is a good start, but it's too big. You should cut out lots of it. Only the essentials.)
  3. Why a better solution is desirable. Who is the "customer" for the better way?

Section: Description
Now describe your feature.

Section: Drawbacks, Alternatives (drawbacks, i.e. breakage, language complexity, corner cases)
Here you want to be as honest as possible. Try to argue the opposition's case for them. Imagine you're a language designer trying to find reasons not to include yet another feature. List all known viable alternatives. Libraries? Wrappers?

That's what I've got.

@zachthemystic
Copy link
Contributor

From the Rationale section:

In fact, there is currently no safe compiler-agnostic way to create a function that can be passed to a delegate. This proposal solves that.

There are really two very different questions here.

  1. How important is it to solve this problem, regardless of how it is solved?
  2. Assuming it needs to be solved, how do you solve it?

Each point should be raised and debated separately, not conflated. The title of the DIP is Delegateable Functions. That's what this DIP is really about. Choosing the specific semantics is secondary compared to that.

@mdparker
Copy link
Member

mdparker commented May 27, 2017

Alright, sorry for keeping you waiting @marler8997. I'm ready to get this moving now.

Unfortunately, it's not currently in a state that I'm comfortable assigning it a number and merging. It needs to be fleshed out some more first. Keep in mind the goal at this stage is to fill in all of the obvious holes, and any not-so-obvious ones we happen to uncover, before moving on to the preliminary review in the forums. Ideally, we only want one preliminary review round, so the fewer modifications we need after the first round, the better.

For starters, you've already received some excellent suggestions for enhancement.

  • Using this in the context proposed by the DIP is unusual and I can promise you it will come up again in the review. extern(delegate) is an obvious alternative. This really needs to be addressed somewhere in the Description. If there is any reason at all why the former is superior to the latter, spell it out. At a minimum, you should lay out the advantages and disadvantages you see in each (as you mentioned in an earlier comment here).

  • The rationale is too short. It reads more like an abstract. Your ultimate goal here is to convince Walter and Andrei that this proposal will add value to the language. The rationale is where you really sell it to them. Currently, all it really does is state the obvious: functions can be called like member functions thanks to UFCS and functions can't be passed as delegates because of ABI issues, so this proposal fixes it. The lingering question: why is this a problem in the first place? How does this feature make D a better language? Identify the need and justify the solution.

  • Nowhere do I see mention of std.functional.toDelegate. I realize toDelegate is not @safe and I assume that you are wanting something that can be used in @safe code, but that needs to be explicit in the DIP.

These, as I see it, are the most obvious holes at the moment. Address these issues and we'll be much closer to getting it merged.

On the cosmetics....

I agree with the recommendations to move the rationale to the top of the description. It makes more sense. The text before the Rationale can be labeled "Details". I'll modify the DIP template to match.

I'll save my copy editing for later, but I'd like to point out two things now. All mentions of "this" should be replaced with this. And please don't use caps for emphasis. Use bold. (MUST NOT vs must not).

@marler8997
Copy link
Contributor Author

@mdparker Thank you for the suggestions and taking the time to review. They are good improvements and I will work on integrating them into the DIP.

@marler8997 marler8997 changed the title Delegateable Functions extern(delegate) Jun 23, 2017
@marler8997
Copy link
Contributor Author

@mdparker After thinking about it for a while I came to the conclusion that the most likely syntax to get accepted will be extern(delegate) (thanks to @skl131313 for his input on this). I've updated the DIP to use this syntax.

I've updated the Rationale section. It features the most prominent use case for this feature and walks through the current solutions and how they compare to the proposed one. It also mentions the difference between this proposal and the toDelegate function.

I was a little unclear on how you wanted me to change the order of the sections? I wasn't sure if you were telling me to move the Rationale section before the description, that seems odd since the Rationale section doesn't make sense unless you read the Description first, but I think you meant something else.

@mdparker
Copy link
Member

Thanks for the update. I'll review it sometime this weekend.

I wasn't sure if you were telling me to move the Rationale section before the description

That's exactly what I meant!

that seems odd since the Rationale section doesn't make sense unless you read the Description first, but I think you meant something else.

Generally, I think it should be the opposite -- before reading the how to make the changes, one should first understand why the changes are needed (and the new template reflects that). The fact that your Rationale (skimmed it just now) doesn't make sense coming before the Description is because you've written it from the perspective that it comes after. Putting it before would require a bit of revision.

Given that, and how much work you've put into it so far, we can just leave it where it is.

I'll let you know by the end of the weekend where we stand with this. Thanks!

@marler8997 marler8997 closed this Jun 24, 2017
@marler8997
Copy link
Contributor Author

@mdparker I'm not sure why github closed this PR, but it's not letting me reopen it, are you able to?

@marler8997
Copy link
Contributor Author

Oh well, I just created a new one here #74

@mdparker
Copy link
Member

Yeah, sorry, I'm not able to reopen it either. I thought you had closed it. I'm sure the message I got indicated you had closed it. Wonder what happened.

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.

9 participants