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

Champion "IntPtr operators" #48

Closed
1 of 5 tasks
MadsTorgersen opened this issue Feb 9, 2017 · 19 comments
Closed
1 of 5 tasks

Champion "IntPtr operators" #48

MadsTorgersen opened this issue Feb 9, 2017 · 19 comments
Assignees

Comments

@MadsTorgersen
Copy link
Contributor

@MadsTorgersen MadsTorgersen self-assigned this Feb 9, 2017
@gafter gafter added this to the 7.2 candidate milestone Mar 19, 2017
@Unknown6656
Copy link
Contributor

I would like to propose four more operators:

implicit unsafe operator void*(IntPtr);
implicit unsafe operator void*(UIntPtr);
implicit unsafe operator IntPtr(void*);
implicit unsafe operator UIntPtr(void*);

as I am often working with a mixture of P/Invoke-calls (which do not care whether I use void* or IntPtr as they are handled identically), CLR-exposed APIs (which use IntPtr) and pointers.
This would greatly reduce the need of writing (void*)... or (IntPtr)... inside my code and would add little effort on the LDT-side, as the implementation of these operators are one-liners.

@nietras
Copy link

nietras commented Apr 8, 2017

For your consideration I have also done some work on implementing nint/nuint as thin struct wrappers (written in IL) to expose this API in https://github.com/DotNetCross/NativeInts but getting proper compiler support for all operations would be much better, and it would allow for checked/unchecked behaviour to be handled correctly. nint is unchecked by default in my implementation.

@nietras
Copy link

nietras commented Apr 8, 2017

I would suggest adding the following implicit cast operators , since these do not loose precision.

implicit operator long(System.IntPtr)
implicit operator System.IntPtr(int) 
implicit operator ulong(System.UIntPtr)
implicit operator System.UIntPtr(uint)

Not sure I understand the need for the two below. Why are these needed? To support explicit cast?

explicit operator System.IntPtr(System.IntPtr)
explicit operator System.UIntPtr(System.UIntPtr)

@nietras
Copy link

nietras commented Apr 8, 2017

I would also add comparison support for long and ulong e.g. IntPtr can be compared to long (which it can) and similarly for UIntPtr and ulong. There are scenarios where this is interesting, of course you could then cast to e.g. long before but that seems unnecessary and cumbersome. Any reason to not include these?

@Unknown6656
Copy link
Contributor

@nietras Concerning your last comment: We will not need to implement comparison operators explicitly, if the implicit-cast-operator from/to (u)long is implemented.

@nietras
Copy link

nietras commented Apr 9, 2017

not need to implement comparison operators explicitly, if the implicit-cast-operator from/to (u)long is implemented.

@Unknown6656 yes, of course 👍 😄

@tannergooding
Copy link
Member

@nietras, comparison of long/ulong to IntPtr/UIntPtr is explicitly not allowed by the CLI:
image

As such, comparison operators should not be provided and I would further think that implicit conversion should not be allowed.

@nietras
Copy link

nietras commented Apr 10, 2017

@tannergooding this table also says you can't compare long to int which of course you can 🙄

long l = 1;
int i = 2;
bool b = l > i;

As far as I'm concerned, and I'm no expert, the intent of this table is not to say that you can't compare a native int with an int64, you just can't do it directly, so you need to convert first. Just as for int32. Why? Because, this way you explicitly tell the CLI how the comparison should be done i.e. in i8 with a conv.i8. So this works fine:

    .method public hidebysig specialname static 
            bool  op_GreaterThan(valuetype DotNetCross.NativeInts.nint l,
                                int64 r) cil managed
    {
        .maxstack  2
        ldarg.0
        ldfld      native int DotNetCross.NativeInts.nint::Value
        conv.i8
        ldarg.1
        cgt
        ret
    } // end of method nint::op_GreaterThan

as you can see in https://github.com/DotNetCross/NativeInts/blob/master/src/DotNetCross.NativeInts/NativeInts.il#L781

Whether or not we then wan't implicit or explicit conversions should be based on the same guideline as for int to/from long in my view. So do we loose precision? If not, implicit is fine.

I do, however, understand that IntPtr might be viewed differently due to the historic Ptr in it's name. But since we would allow arithmetic on IntPtr, I do not see how converting implicit to long can be seen as a bad thing. Adding 17 to a pointer to a double is much worse. 😉 And I would simply ignore the Ptr part of this, it should have been called nint, but that is the past.

@nietras
Copy link

nietras commented Apr 12, 2017

I see this has been labelled as "7.2 Candidate" was hoping for 7.1, any way we/I can help speed this up? I see next step is prototype...

@jkotas
Copy link
Member

jkotas commented Apr 17, 2017

Alternative design: #435 dotnet/corefxlab#1471

@MichalStrehovsky
Copy link
Member

Alternative design: #435 dotnet/corefxlab#1471

I think the support for some of the operators on IntPtr would be valuable even if the other proposal goes through. There are many valid scenarios where I e.g. want to set the lowest bit in a pointer. Having to cast the IntPtr to a "pointer-sized integer" just so that we can set/clear the lowest bit is the same amount of annoyance as having to cast to long. Sure, it will be more efficient with a "native sized int", but it won't look any better - I want to be able to this as a "pointer operation" as opposed to an "arithmetic operation on an integer type".

@sharwell
Copy link
Member

sharwell commented Apr 19, 2017

I notice the proposal does not explicitly state that the proposed operators should be "predefined operators" specifically for the C# language. The operators themselves do not need to be defined in metadata. This could be inferred by readers familiar with C# and the underlying IL, but it would still help to clarify it in the proposal. The distinction is particularly important for the compile-time handling of /checked[+ | -].

Also, the proposal fails to detail the behavior of checked and unchecked arithmetic with respect to the new operators.

@tannergooding
Copy link
Member

tannergooding commented Apr 19, 2017

Yes. The proposal could definitely be more explicit on those points.

I intended these operators to be implemented entirely by the compiler (as is done for the other runtime primitives) and for checked/unchecked handling to be treated the same.

All of this is supported and clearly defined by the CLI spec, its just that the original language design (for whatever reason) decided against including the support for this primitive type (and only this one).

@gafter
Copy link
Member

gafter commented Apr 19, 2017

I intended these operators to be implemented entirely by the compiler (as is done for the other runtime primitives) and for checked/unchecked handling to be treated the same.

How would the compiler know whether the following is an error (due to overflow) or not?

    const IntPtr x = (IntPtr)int.MaxValue + (IntPtr)1;

@sharwell
Copy link
Member

📝 @tannergooding If you are happy with my explanation of proposed overflow handling in #435 you could copy it here, and/or modify it as you find appropriate.

@gafter
Copy link
Member

gafter commented Apr 19, 2017

@sharwell Please help me understand how your proposal handles this:

const IntPtr a = unchecked((IntPtr)uint.MaxValue + (IntPtr)1);
const bool c = a == (IntPtr)0;

The bool c must be true if run on a 64-bit platform, and false if run on a 32-bit platform. But since it is a compile-time constant, it must be fixed to one of those values before it is ever run on any platform.

@tannergooding
Copy link
Member

I posted the answer on the other thread as well. The CLI spec explicitly defines that native int constants are only allowed to be 32-bit values.

@gafter
Copy link
Member

gafter commented Apr 20, 2017

@tannergooding That does not tell me if unchecked((IntPtr)uint.MaxValue + (IntPtr)1) is disallowed somehow, or merely not a constant.

@MadsTorgersen MadsTorgersen modified the milestones: 7.X candidate, 7.2 candidate Jul 5, 2017
@MadsTorgersen
Copy link
Contributor Author

This is an either/or with native int types #435, which we'd rather do. If we drop those, this will be back in consideration.

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

No branches or pull requests

8 participants