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) Allow const members for value types using default(...) #7592

Closed
jskeet opened this issue Dec 18, 2015 · 20 comments
Closed

(Proposal) Allow const members for value types using default(...) #7592

jskeet opened this issue Dec 18, 2015 · 20 comments

Comments

@jskeet
Copy link

jskeet commented Dec 18, 2015

Currently there is a small disparity between optional parameters and const declarations. While you can have an optional parameter with a default value of new SomeValueType() or default(SomeValueType) you can't use const for non-simple value types. It would be really nice if this were permitted - possibly via some attribute like const declarations for decimal values use - to make default arguments more readable. Compare the intent of:

public void FooAsync(CancellationToken cancellationToken = default(CancellationToken))

and

public void FooAsync(CancellationToken cancellationToken = CancellationToken.None)

The latter doesn't compile, because CancellationToken.None isn't a constant expression - but it could have been. (We couldn't change None to be a const for compatibility reasons, but think of future uses... and a new member could be introduced elsewhere, potentially.)

I don't know whether we should also allow a 0-argument constructor call, as is currently allowed for default argument values... I'm leary of that just in case custom parameterless structs (proposed but then dropped for C# 6) are ever introduced. I think we've already got an issue there with default argument values - I wouldn't want to make it worse.

@leppie
Copy link
Contributor

leppie commented Dec 18, 2015

Maybe the compiler should try emit the code differently for these cases (or IMO all of them). IOW:

public void FooAsync() {
  FooAsync(CancellationToken.None);
}

public void FooAsync(CancellationToken cancellationToken) {
  ...
}

@svick
Copy link
Contributor

svick commented Dec 18, 2015

@leppie How would that work with multiple optional parameters? I.e. something like:

public void Foo(Struct1 param1 = Struct1.Value, Struct2 param2 = Struct2.Value)
{}Foo(param1: myValue);
Foo(param2: anotherValue);

@leppie
Copy link
Contributor

leppie commented Dec 18, 2015

@svick: Good point.

@HaloFour
Copy link

So what exactly is this proposal? A way to decorate a static property of a struct so that the value represented by that property could be considered a blittable constant that can be embedded in the CLR metadata for the optional parameter?

@jskeet
Copy link
Author

jskeet commented Dec 18, 2015

The proposal is for this to be valid, as an example:

public class Foo
{
    public const CancellationToken None = default(CancellationToken);

    public Task FooAsync(CancellationToken cancellationToken = None)
    ...
}

@alrz
Copy link
Member

alrz commented Dec 18, 2015

Combination of #2401, #5474 and #952 (although, for non-enums)?

@aluanhaddad
Copy link

👍

@HaloFour
Copy link

@jskeet Limited specifically to "aliases" of default(T) or also supporting custom values?

The original "proposal" isn't really a proposal as it is identifying a potential shortcoming. I'm trying to get specific syntax and behavior that you'd think would solve the shortcoming.

So this is really two issues. One would be expanding on what is permitted to be embedded as a const. This is largely limited by the CLR to the types that it understands as encoded as BLOBs directly in the assembly metadata. However, the C# and VB.NET compilers both already sort-of workaround this limitation for the decimal and Date data types respectively by converting them into static readonly fields and using well known attributes to encode their binary representation.

Second would be a way to encode that an optional parameter refers to one of these constants. Even if you could define a constant of a specific value (or the default value) it is that value that is embedded as the default value of the parameter, not any reference to the parameter. Otherwise the optional value is ultimately just the value of the constant, not the constant, and callers would still see [cancellationToken = default(CancellationToken)] and not [cancellationToken = None], e.g.

public class Foo {
    public const int None = default(int); // legal

    public Task FooAsync(int token = None); // also legal
}

FooAsync(); // intellisense shows default(int), not Foo.None

@jskeet
Copy link
Author

jskeet commented Dec 19, 2015

@HaloFour: I'm only proposing allowing default(T) for the moment. While I would welcome more general const-ing, that would be a more in-depth proposal.

And yes, it would be good for an optional parameter to encode how its default value was derived (probably via attributes) - although there's accessibility issues there as well (e.g. when generating documentation or using Intellisense, if the caller wouldn't have access to the member in question, then default(T) would probably be best.

@alrz
Copy link
Member

alrz commented Dec 30, 2015

@jskeet I think #7737 syntax, is far better than yet too verbose F(CancellationToken cancellationToken = CancellationToken.None) and it doesn't look awful like F(CancellationToken cancellationToken = default(CancellationToken)), don't you think?

@jskeet
Copy link
Author

jskeet commented Dec 30, 2015

@alrz: well you would think that, given that it's your proposal :) But in the same vein, I prefer mine... Names allow semantics to be expressed clearly.

@alrz
Copy link
Member

alrz commented Dec 30, 2015

@jskeet Back to your proposal, given that None is a property, how would you know that it is a constant? and yet making it a readonly variable doesn't work. Although, I believe if it is marked as a pure property (#7561) the compiler could figure it out.

@jskeet
Copy link
Author

jskeet commented Dec 30, 2015

@alrz: As I've already said, it's too late for CancellationToken.None itself:

We couldn't change None to be a const for compatibility reasons, but think of future uses... and a new member could be introduced elsewhere, potentially.

@alrz
Copy link
Member

alrz commented Dec 30, 2015

While I don't see why None is defined as a property and not just a static readonly variable in the first place, still structs cannot be considered as a constant for some reason, even only for default(T) case. Also, CLR verifier go to great pains to guarantee this for readonly fields and yet they're not reliable. My point is, a feature set around totally immutable objects (#7626) is required to facilitate these use cases (having a const of a struct). Otherwise, I can't see how this could be done.

@jskeet
Copy link
Author

jskeet commented Dec 30, 2015

Why could it not be done? It works for default arguments. Why could the compiler not do the same thing elsewhere? I think I'd like the compiler team to comment on feasibility... But the community is in a good position to discuss desirability.

@alrz
Copy link
Member

alrz commented Dec 30, 2015

It works for default arguments.

I think the proposal actually suggests to make it works for default arguments, meaning that it currently doesn't, right? Or I'm missing something.

Anyway, I think this is the reason behind it:

// OK, compile-time constant, and you cannot define parameterless ctor for structs
void F(Struct s = new Struct())

// OK, same as above
void F(Struct s = default(Struct))

// Error, Default parameter value for 's' must be a compile-time constant
void F(Struct s = new Struct(1))

Now, imagine this for a static readonly field,

// Error, because 'None' could be anything, like 'new Struct()' or 'new Struct(1)'
void F(Struct s = Struct.None)

Are you suggesting that Struct.None becomes a compile-time constant, or we should relax optional arguments to accept non-compile-time constants? Either way, we have to ensure the immutability of the underlying type. This works for int etc because they are inherently immutable and the compiler knows it. But for non-simple value types that is not the case and we don't have any notation to make them so.

@jskeet
Copy link
Author

jskeet commented Dec 31, 2015

No, default(T) already works for default arguments. It's fine to have

void F(CancellationToken cancellationToken = default(CancellationToken))

This proposal would allow named constants with the value of default(T), which would be inlined at the usage site in the same was as other constants are.

I'm not proposing that any static readonly field becomes usable as a constant expression - just default(T). So in other words, this would be valid:

public struct Foo { ... }

public class Bar
{
    public const Foo EmptyFoo = default(Foo);
}

Then any time Bar.EmptyFoo is used (whether in the same assembly or not), the compiler inserts the appropriate IL as if the expression were actualy default(Foo). At that point it doesn't matter whether Foo is immutable or not, as far as I can see. Why do you think it does?

@alrz
Copy link
Member

alrz commented Dec 31, 2015

@jskeet I was talking about your example in the original post all this time, while you meant something like public const CancellationToken Never = default(CancellationToken);. Phew.

@jskeet
Copy link
Author

jskeet commented Dec 31, 2015

@alrz: Yes, the example in the original post is how nice it could have been if this feature had been introduced from the start. I explicitly specified that it's too late for CancellationToken.None... do you now see what I've been getting at?

One wrinkle I hadn't previously considered: we'd need to prohibit the use of user-defined operators for non-built-in types for constants (whereas we need to continue to allow them for decimal constants).

@gafter
Copy link
Member

gafter commented Mar 24, 2017

Issue moved to dotnet/csharplang #336 via ZenHub

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

7 participants