dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

Proposal: Support comparisons of IntPtr and UIntPtr #18311

Closed jamesqo closed 4 years ago

jamesqo commented 8 years ago

Rationale

Now that dotnet/runtime#17305 has been approved meaning both of IntPtr and UIntPtr will implement IEquatable<>, it would also make sense for them to implement IComparable<> and the comparison operators as well. This will amend this rather unintuitive situation, where it is possible to compare 2 raw pointers with <, but not 2 IntPtrs. It will also add better support for native math since it will allow two words to be compared without manually calling ToPointer() on each of them (and therefore resorting to unsafe code). Finally, implementing IComparable means Array.Sort on an array of IntPtrs will just work as expected, without having to write your own comparer.

Proposed API

namespace System
{
    public struct IntPtr : IEquatable<IntPtr>, IComparable<IntPtr>
    {
        public static bool operator <(IntPtr left, IntPtr right);
        public static bool operator <=(IntPtr left, IntPtr right);
        public static bool operator >(IntPtr left, IntPtr right);
        public static bool operator >=(IntPtr left, IntPtr right);

        public int CompareTo(IntPtr other);
    }

    public struct UIntPtr : IEquatable<UIntPtr>, IComparable<UIntPtr>
    {
        public static bool operator <(UIntPtr left, UIntPtr right);
        public static bool operator <=(UIntPtr left, UIntPtr right);
        public static bool operator >(UIntPtr left, UIntPtr right);
        public static bool operator >=(UIntPtr left, UIntPtr right);

        public int CompareTo(UIntPtr other);
    }
}

Open Questions

var ptr = new IntPtr(-1);
Console.WriteLine(ptr.ToInt64() < IntPtr.Zero.ToInt64()); // True
Console.WriteLine(ptr.ToPointer() < IntPtr.Zero.ToPointer()); // False
Console.WriteLine(ptr < IntPtr.Zero); // What should this be?

After dotnet/coreclr#6855 I'm guessing the better answer is the first one.

Related issues

jamesqo commented 8 years ago

@omariom @KrzysztofCwalina @jkotas @GSPP @tannergooding

jkotas commented 8 years ago

Should IntPtr have the same semantics as int/long (signed) or void* (treated as unsigned?) for comparisons

Right, it is confusing either way.

This needs careful thought about design and usability. I am not sure whether bolting operators on top IntPtr is the best design. It may be better to invent a new "native int / native uint" type - like what is proposed in https://github.com/dotnet/coreclr/issues/963.

GSPP commented 8 years ago

Isn't it a natural solution to treat IntPtr and UIntPtr like the corresponding native integer types that the CLR supports? That's a clear definition for semantics. It's also a clear guideline for completing the operators and language features - just make these types look like normal integers.

IntPtr is signed, then. User mode addresses have the high bit cleared so that's almost never an issue. It would come into play for differences of pointers. For differences, signedness is desired. So the more common IntPtr already behaves as desired in most cases.

jamesqo commented 8 years ago

It may be better to invent a new "native int / native uint" type

Hmm, but isn't IntPtr already represented in IL as a native int? If we were to introduce a new, say, nint keyword that mapped also mapped to native int, and then had a library like

public static nint GetAvailableMemoryForSomething();

and then used it from another library, how would we be able to tell whether the method was returning IntPtr or nint?

tannergooding commented 8 years ago

@jamesqo, as for your open question. The ECMA-335 spec for the CLI clearly defines the operators and behaviors of the operators on the native int and unsigned native int types. I would believe we should follow the same behaviors here.

@jkotas, what use would a new native int type provide? The IntPtr class is the current primitive wrapper for the native int type. The CLI defines a clear set of supported operators and the CLR supports them.

The only remaining issue, seems to be, that some languages, like C#, do not treat IntPtr as a primitive type and therefore do not support the operators defined by the CLI.

Adding the operators defined by the CLI seems fairly straightforward and is merely defining operators that the CLI defines but declares as 'optional' for language support.

jkotas commented 8 years ago

User mode addresses have the high bit cleared so that's almost never an issue

User mode addresses do not always have the high bit cleared. It is an issue for both Windows and Linux. For example, check https://github.com/dotnet/corefx/issues/11085 fixed a few days ago.

jkotas commented 8 years ago

IntPtr already behaves as desired in most cases

I have tried to use IntPtr as native int with operator overloading in corelib. I was not happy with how it worked, For example - IntPtr -> int cast operator is checked, different from what one is used to, you have to keep reminding yourself about the differences. I was much happier with alias under ifdef, like https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Buffer.cs#L18. The down-side is that the alias works only if the code is compiled platform-specific. The ideal solution would be to have usability you get with alias trick, but without the need to compile platform-specific.

tannergooding commented 8 years ago

@jkotas, I think, for most of these (equality, comparison, and simple math) the behaviors are not unusual and are well defined. The cases where it is (such as casting to int) we already have a friendly operator (ToInt32) and an explicit operator may not be needed.

However, cases where you need to cast should also go down/away when we add the remaining operators, as native int should work (without casting) for the majority of the use cases where you would cast today.

omariom commented 8 years ago

May be new types NInt and NUInt with conversation operators to IntPtr and UIntPtr?

tannergooding commented 8 years ago

@omariom, I'm still not sure what benefit that would bring. The new types would have the exact same 'confusion' that modifying IntPtr would have, and we would be adding a new type that needed special handling to treat it as a primitive type, work with marshalling in a special way, etc.

Adding the operators to System.IntPtr wouldn't be a breaking change, since it has none now. It would merely be fleshing out the surface area to match what is supported by the CLI.

AlexGhiondea commented 7 years ago

We have a formal API that we can review. Marking it accordingly.

@terrajobst can you add this to the review schedule?

terrajobst commented 7 years ago

We're reviewing all issues marked as api-ready-for-review once a week. Next week, we'll do a two hour round so that we can unblock more issues for our holiday hackers :-)

karelz commented 7 years ago

API review: Rejected - We should not treat pointers as numbers.

tannergooding commented 7 years ago

These points may have been brought up in the API review already, but...

I personally don't feel like this would be treating them as numbers. This would be treating them as a pointer to an address in memory (which they are). And one address can be greater than or less than another address.

These are just standard pointer arithmetic operations, which are supported by the IL but is not currently exposed to 'some' languages (such as C#).

karelz commented 7 years ago

The argument during API review was that, once you add operators (<, >=, etc.), we should add also +, -. That's a slippery slope.

My personal view: I don't understand how I could use comparison <, >= on IntPtrs without being unsafe. The IntPtrs have to be either native, or pinned. In both cases I already crossed the boundary into unsafe land. Or is there other scenario where comparison makes sense?

omariom commented 7 years ago

Why treat IntPtr as a pointer? Isn't it an integer of pointer size? Pointers are unsafe. Not integers.

tannergooding commented 7 years ago

I agree that the majority case here is likely for unsafe/interop code. However, not all interop code that works with native sized integers involves pointers.

For example, size_t is a platform size integer, where arithmetic may be needed, but it is strictly not a pointer.

Imagine that I have written a C# library that wraps some graphics library written in C/C++ (such as DirectX, OpenGL, or Vulcan). I might have a C structure declared like so:

struct SUBRESOURCE_DATA
{
    const void* pData;
    SIZE_T RowPitch;
    SIZE_T SlicePitch;
}

To be able to interact with the structure in C#, I would then define:

unsafe internal struct SubresourceData
{
    public void* pData;
    public IntPtr RowPitch;
    public IntPtr SlicePitch;
}

To access a particular slice of the subresource, you would do pSliceStart = pData + SlicePitch * SliceIndex, and then to access a particular row you would do pRowStart = pSliceStart + RowPitch * RowIndex.

I believe it is completely reasonable to want to calculate an offset using basic integer arithmetic on RowPitch or SlicePitch which represent a size/offset and not an actual pointer into memory (which is represented by pData).

The CLI spec states that IntPtr and UIntPtr are the primitive wrappers for native int and unsigned native int and further declares the full set of operators that these types support, which includes those requested in this issue and the other standard arithmetic operators. The request here is to just expose those to non IL code so that users can interact with these types as may be required for their code (without having to go and write IL or perform crazy casting which makes the code confusing and obfuscates the actual intent of a field).

The remarks section of the System.IntPtr even indicates the following:

The IntPtr type is designed to be an integer whose size is platform-specific. That is, an instance of this type is expected to be 32-bits on 32-bit hardware and operating systems, and 64-bits on 64-bit hardware and operating systems.

tannergooding commented 7 years ago

If adding these operators to System.IntPtr is truly out of scope (due to IntPtr only being meant to represent pointers/handlers and not sizes/offsets). Then I believe a new proposal should be opened, one which adds a type meant to represent a platform sized integer and not a pointer/handle.

jkotas commented 7 years ago

new proposal should be opened, one which adds a type meant to represent a platform sized integer and not a pointer/handle.

These proposal exists in various forms already. https://github.com/dotnet/roslyn/issues/2788, https://github.com/dotnet/coreclr/issues/963 and related issues.

I agree that the lack of platform sized integer is a pain when writing interop or low-level code.