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

nint/IntPtr differences should be disallowed in partial methods #45301

Closed
RikkiGibson opened this issue Jun 18, 2020 · 6 comments
Closed

nint/IntPtr differences should be disallowed in partial methods #45301

RikkiGibson opened this issue Jun 18, 2020 · 6 comments

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Jun 18, 2020

Related to #45128

Version Used: 472c072

Steps to Reproduce:

using System;

partial class C
{
    partial void M1(IntPtr x);
    partial void M1(nint x) { }

    public partial IntPtr M2();
    public partial nint M2() => 0;
}

Expected Behavior:
Errors on parameter/return type differences in the implementing declarations.

Actual Behavior:
No diagnostics, the program is allowed to compile and the defining declaration is used in the metadata signature.

/cc @cston. Native integer spec changes may be needed.

@jaredpar jaredpar added the Bug label Jun 22, 2020
@jaredpar jaredpar added this to the 16.8 milestone Jun 22, 2020
@jaredpar
Copy link
Member

This needs to go through LDM before we make the change because the proposed design goes against how partial methods work for every other type. For example it's okay to mix dynamic and object in the partial signatures hence it should follow that we can mix nint and IntPtr.

I agree this design seems a bit off and if we were starting from scratch I would argue against it. At this point though this is the design that we are working with. Trying to enforce a new design for nint only seems like it will just make the situation more confusing for developers, not better.

@cston
Copy link
Member

cston commented Jun 22, 2020

Trying to enforce a new design for nint only ...

The compiler currently reports errors for differences in tuple element names (see example).

@jaredpar
Copy link
Member

@cston yes it does but that seems the outlier behavior here. If we made any change here it would seem the more sensible change would be to remove that warning. Adding more deviations just makes the behavior harder and harder to explain. At some point it becomes more like a trivia question than a design.

@RikkiGibson
Copy link
Contributor Author

We may have reached the point in the schedule where this needs to become a future warning wave instead of an error.

@cston
Copy link
Member

cston commented Aug 21, 2020

See #45519 for the set of differences in signature that should be reported as errors.

@cston
Copy link
Member

cston commented Sep 18, 2020

Added a warning wave warning for partial method type differences in #47576.

@cston cston closed this as completed Sep 18, 2020
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

3 participants