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

[Proposal]: Relaxing shift operator requirements #4666

Open
4 tasks done
Tracked by #829 ...
tannergooding opened this issue Apr 17, 2021 · 15 comments
Open
4 tasks done
Tracked by #829 ...

[Proposal]: Relaxing shift operator requirements #4666

tannergooding opened this issue Apr 17, 2021 · 15 comments
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Milestone

Comments

@tannergooding
Copy link
Member

tannergooding commented Apr 17, 2021

Relaxing shift operator requirements

Summary

The shift operator requirements will be relaxed so that the right-hand side operator is no longer restricted to only be int.

Motivation

When working with types other than int, it is not uncommon that you shift using the result of another computation, such as shifting based on the leading zero count. The natural type of something like a leading zero count is the same as the input type (TSelf) and so in many cases, this requires you to convert that result to int before shifting, even if that result is already within range.

Within the context of the generic math interfaces the libraries are planning to expose, this is potentially problematic as the type is not well known and so the conversion to int may not be possible or even well-defined.

Detailed design

https://github.com/dotnet/csharplang/blob/main/spec/expressions.md#shift-operators should be reworded as follows:

- When declaring an overloaded shift operator, the type of the first operand must always be the class or struct containing the operator declaration, and the type of the second operand must always be int.
+ When declaring an overloaded shift operator, the type of the first operand must always be the class or struct containing the operator declaration.

That is, the restriction that the first operand be the class or struct containing the operator declaration remains. While the restriction that the second operand must be int is removed.

Drawbacks

Users will be able to define operators that do not follow the recommended guidelines, such as implementing cout << "string" in C#.

Alternatives

The generic math interfaces being exposed by the libraries could expose explicitly named methods instead. This may make code more difficult to read/maintain.

The generic math interfaces could require the shift take int and that a conversion be performed. This conversion may be expensive or may be not possible depending on the type in question.

Unresolved questions

Is there concern around preserving the "intent" around why the second operand was restricted to int?

Design meetings

@tannergooding
Copy link
Member Author

Is there concern around preserving the "intent" around why the second operand was restricted to int?

The right hand side could be restricted to be some INumber<TSelf> where TSelf : INumber<TSelf> type so that it is always an integral.

That being said, nothing is restricting a user from defining operator / for use as the path joining operator (which likewise follows what C++ does in its STL) and so the belief is that the framework design-guidelines and best practices will help prevent users from doing the "wrong" thing.

Additionally, the language will already consume operators (such as defined in IL) where the second operator is not int. Operators can even be "defined" in C# using the System.Runtime.CompilerServices.SpecialNameAttribute and so this is largely just an artificial limitation of the language:

image

@tannergooding
Copy link
Member Author

CC. @MadsTorgersen, @AlekseyTs, @stephentoub

Same here. Please let me know if you believe anything from the meeting was not covered here or if there are any additional changes or clarifications that I can make to the proposal.

@Unknown6656
Copy link
Contributor

Unknown6656 commented Apr 22, 2021

Operators can even be "defined" in C# using the System.Runtime.CompilerServices.SpecialNameAttribute and so this is largely just an artificial limitation of the language.

Hm, I could not get the following code to compile on sharplab:

using System.Runtime.CompilerServices;
using System;

C x = new();
x = x << 3;
//  ^^^^^^ error CS0019: Operator '<<' cannot be applied to operands of type 'C' and 'int'

class C
{
    [SpecialName]
    public static C op_LeftShift(C c, object o)
    {
        Console.WriteLine("shift operator called with " + o);
        
        return c;
    }
}

Am I missing something?

https://sharplab.io/#v2:EYLgtghglgdgNAFxAJwK7wCYgNQB8ACADAAT4CMAdAEroJRgCmFAwgPZgAOUANg8gMp8AblADGDAM4BuALAAofACYy81XObEAHsQC8xGAwDuACgCUsudr3aAPDeIBmC2qXFm8gN7ziP4gG1+DgZRKAhuADkIRgBdb198B1IyADY3YlYOAH0AGQYAMwR+AAsoAuMNUTh04AArYIR00zifLzlfdqSATmMAIgkSgvSg5AgEVmRiUTDeDGJDKAQi4h7ibEaLDt9mjvwAdkmN3wBfeSOgA===

@tannergooding
Copy link
Member Author

tannergooding commented Apr 22, 2021

It cannot be within a single assembly, it must be across two separate assemblies

That is, if assembly A defines [SpecialName] public static C op_LeftShift(C c, object o), then assembly B can consume it, but A itself cannot due to resolving from source vs resolving from metadata

@Unknown6656
Copy link
Contributor

Ahh, I see. Thanks for clearing that one up!

@naine
Copy link

naine commented Feb 9, 2022

The only alternatives I see mentioned are workarounds for not changing the language at all. Has a lesser feature been considered where the restriction is only partially relaxed?

For example, rather than removing restrictions on the type of the second operand entirely, the restriction could be relaxed to "any primitive integer type or the type containing the operator declaration". I think this would fulfil the needs of the generic math interfaces while preserving the intent behind the original restriction.

@CyrusNajmabadi
Copy link
Member

For example, rather than removing restrictions on the type of the second operand entirely, the restriction could be relaxed to "any primitive integer type

This would not work for any integer type that wasn't a primitive. For example, say you want to be shiftable by Int128 (which maybe is only a library type and not a primitive type).

@naine
Copy link

naine commented Feb 9, 2022

I can't imagine a use case for that except when the type being shifted is the same type. Within the Int128 type itself that would be accepted due to the "or the type containing the operator declaration" clause (the same restriction imposed on the first operand). But it does not make much sense to want to declare:

struct CustomInt
{
    public static CustomInt operator <<(CustomInt a, Int128 b);
}

@tannergooding
Copy link
Member Author

tannergooding commented Feb 9, 2022

The amount of work required to restrict it is likely more than the amount of work required to simply remove the restriction.

Users can always misuse features. You can define a operator /(Path x, Path y) today and use it for concatenation like C++ allows. You can decide to write all kinds of patterns and complex expressions that make code harder to read/maintain/etc

Most users (and particularly the most popular libraries) will not however. And even if C# did restrict what it could define, it already accepts and utilizes the operators some other language may define that don't follow its own rules. So it remains an artificial restriction in what C# can define vs what C# will actually consume from IL.

@naine
Copy link

naine commented Feb 9, 2022

Fair enough. Just wanted to see that it has been considered.

@jcouv
Copy link
Member

jcouv commented Apr 28, 2022

Feature was merged into 17.3p2

@jeffhandley
Copy link
Member

@MadsTorgersen / @tannergooding Can this issue be closed now?

@tannergooding
Copy link
Member Author

Issues on csharplang tend to stay open for some time after they have been implemented and shipped. I'd expect the proposal champion will close it as appropriate for the C# LDM needs.

@jcouv jcouv modified the milestones: Working Set, 11.0 Sep 26, 2022
@333fred 333fred added the Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification label Jan 9, 2023
@jrmoreno1
Copy link

jrmoreno1 commented Apr 8, 2023

@333fred : could you update the check marks for how far along it is?

@333fred
Copy link
Member

333fred commented Apr 10, 2023

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Projects
None yet
Development

No branches or pull requests

9 participants