csharplang icon indicating copy to clipboard operation
csharplang copied to clipboard

Champion "Target-typed `new` expression" (VS 16.8, .NET 5)

Open gafter opened this issue 8 years ago • 204 comments
trafficstars

Summary: Allow Point p = new (x, y);

Shipped in preview in 16.7p1.


  • [X] Proposal added
  • [X] Discussed in LDM
  • [X] Decision in LDM (let's go ahead with prototype)
  • [ ] Finalized (done, rejected, inactive)
  • [ ] Spec'ed

See also https://github.com/dotnet/roslyn/issues/35

  • https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-05-21.md
  • https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-06-25.md
  • https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-08-22.md#target-typed-new
  • https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-10-17.md
  • https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-10-17.md#target-typed-new

Test plan: https://github.com/dotnet/roslyn/issues/28489

gafter avatar Feb 14 '17 23:02 gafter

At least this can be done with object initializers without type-name, which currently generate anonymous types.

gulshan avatar Feb 15 '17 08:02 gulshan

I am more fond of var everywhere instead of empty new

Thaina avatar Mar 26 '17 15:03 Thaina

@Thaina I absolutely agree for locals, but for fields and properties this would actually be preferable to me and a_m_a_z_i_n_g, not gonna lie.

private readonly Dictionary<(Foo foo, Bar bar), List<Baz>> bazByFooAndBar = new();

Love it! Plus also:

private readonly Dictionary<Foo, Bar> barByFoo;

public ctor(IDictionary<Foo, Bar> initialValues)
{
   barByFoo = new(initialValues);
}

I do a lot of field initialization requiring access to this, so I end up saying new and repeating long types quite frequently in constructor bodies.

jnm2 avatar Mar 26 '17 17:03 jnm2

@jnm2 Well, readonly in constructor seem great

But still that the only place I think we should do like that, I don't like mutable reference so field should not new anywhere. I wish we could have var field instead

Thaina avatar Mar 26 '17 17:03 Thaina

@Thaina

I wish we could have var field instead

I believe this is a limitation of the fact that C# has a two-pass compiler. The entire signature of a class needs to be known before any of the initialization expressions can be executed, so if you use var for a field the compiler doesn't know the type on the right-hand side of the =.

Richiban avatar May 22 '17 14:05 Richiban

I think private var fields (and let for readonly) are ok, as long as they are immediately initialized, just like what we have for local vars.

alrz avatar May 22 '17 18:05 alrz

I think private var fields (and let for readonly) are ok, as long as they are immediately initialized, just like what we have for local vars.

@alrz A var-declared local's initialization can only rely on earlier-declared locals. That eliminates the possibility of cycles. Fields do not have any corresponding restrictions that would eliminate such cycles by defining an order in which the types are inferred.

In any case, that would be a completely separate language proposal.

gafter avatar May 22 '17 18:05 gafter

Yes please, let's make things less verbose whenever possible and whenever it make sense. :)

iam3yal avatar May 23 '17 01:05 iam3yal

I'm not sure is it related with this topic.

public List<Error> ValidateInput() => new List<Error>();

This piece of code could be, with some generalization, written as

public List<Error> ValidateInput() => new();

I really don't know is it readable and right direction. Possibly not. Just to leave a little note.

gordanr avatar May 23 '17 19:05 gordanr

public List<Error> ValidateInput() => new();

I'm a bit riven by this. On one hand I think that a method's body should be independent of its signature, meaning that the creation of an object for a return should be verbose. On the other hand for expressions, so expression method bodies as well, this is very neat. It's like default vs. default(T), but I think default is valid in block bodies as well?

lachbaer avatar May 24 '17 07:05 lachbaer

Constructors are kind of useless at the moment, you can't do type inference with them, can't pass a bound constructor as a first class function, etc. etc. At the same time you're stuck using them if you want to use readonly or getter-only properties. The end result is that every class I write has a static "constructor" function that simply delegates to the regular constructor, but behaves like a normal function from the perspective of consuming code.

It would be great if there could be some focus on bringing constructor functions into parity with plain old static methods, instead of piecemeal workarounds like this.

masaeedu avatar Jun 02 '17 18:06 masaeedu

Reading the raw notes:

It may be that there's a subset of the feature that's simpler. We would need that subset to not preclude going further later.

Looking at the concern that an API adding a new overload could break consumers, the "simple" subset of places where target-typed new can be applied (that I'm pretty sure won't preclude further options) would be

  • Local variables
  • Field/property intializers
  • (maybe) return statements

The first two are the places where it seems like most people are really clamoring for it; the last one I just came up at the spur of the moment.

Joe4evr avatar Dec 06 '17 11:12 Joe4evr

@Joe4evr Why would you want target-typed new for locals? There's var already. I would rather use it in function arguments.

orthoxerox avatar Dec 06 '17 21:12 orthoxerox

@orthoxerox for example when the declaration and the construction are not in the same scope:

List<string> list;

try
{
    list = service.GetList();
}
catch (Exception)
{
    list = new (); // <---
}

quinmars avatar Dec 06 '17 21:12 quinmars

@quinmars I was trying to solve that backward by #249

Like this

var list = :{
    try
    {
         return service.GetList();
    }
    catch
    {
         return new List<string>();
    }
}

There is also #616

Like this

var list = try service.GetList() catch new List<string>();

Thaina avatar Dec 07 '17 01:12 Thaina

Notes to self to remember to test or address (I'll keep adding entries):

  • [ ] how should NormalizeWhitespace behave? Is it new(1, 2) or new (1, 2)?
  • [ ] Neal mentioned that new would be a typeless expression. So it would not contribute to overload resolution. It can convert to certain types. We'll need to confirm.
  • [ ] using without argument (new vs. new ())

jcouv avatar Dec 11 '17 19:12 jcouv

how should NormalizeWhitespace behave? Is it new(1, 2) or new (1, 2)?

My 2c: it should be "new(1, 2)" :)

CyrusNajmabadi avatar Dec 11 '17 20:12 CyrusNajmabadi

There is a prototype available here: https://github.com/alrz/roslyn/commit/63f1bb7ad72be94e5668ee192d0e48d2a7a0b2d4

re open questions, if we define the the conversion such that it only depends on the constructor overload resolution (i.e. a conversion exists iff there exists an applicable constructor wrt provided arguments), we can bind initializers after the conversion succeeded.

/cc @jcouv

alrz avatar Feb 02 '18 00:02 alrz

@alrz Would this prototype also cover initialization of class member fields, I didn't see that in the unit tests?

HakanL avatar Feb 02 '18 18:02 HakanL

@HakanL Yes, it's not well tested yet but you can check out this one. constructor args are undone though.

alrz avatar Feb 02 '18 19:02 alrz

Thanks for pointing that out. I guess I was looking for this type of pattern:

   public class X
   {
       MyClass abc = new("Hello");
   }

Where I wouldn't have to enter MyClass twice (like today), essentially what you use for var inside methods. Please correct me if I misunderstood the scope of this ticket.

HakanL avatar Feb 02 '18 19:02 HakanL

Well, that pattern is the primary motivator for this entire proposal to begin with, so I'd be shocked if that didn't work.

Joe4evr avatar Feb 02 '18 19:02 Joe4evr

I was hoping that was the case, but I didn't see that pattern in the unit tests so that's why I asked.

HakanL avatar Feb 02 '18 19:02 HakanL

@HakanL Ah, I thought you mean object initializers (like new() { Property = e }). type-level initializers should definitely work (edit: just confirmed that it's passing. https://github.com/dotnet/roslyn/commit/127b6ed95a504ef36754bf1f6e7bd07b6a59b5c5)

alrz avatar Feb 02 '18 19:02 alrz

@alrz Fantastic! Would you write a small spec (on csharplang) for the feature, for review in LDM? (example) (I set myself a reminder to bring it up at next opportunity). We're going to find/assign someone from the team to be a secondary owner for the feature, who will help with review and open issues.

Discussed with @gafter to try and identify some interesting scenarios. We'd propose that initializers not be supported at first, because they cause a strange order of binding (bind the target-typed-new without initializer, then do overload resolution, then go back and bind the initializer).

Some more scenarios worth testing:

  • [ ] various "best type" scenarios (we expect those will fall out naturally):
    • [ ] M(new C(), new()) with void M<T>(T t1, T t2)
    • [ ] new[] { new C(), new()) }
    • [ ] C c = ...; return c ?? new();
    • [ ] lambda with multiple returns () => { if (b) return new C(); else return new(); }
  • [ ] Neal suggested we may want to disallow new() for primitive struct types (int i = new();), but maybe that's not worth a special language rule but should be handled by analyzer/fixer/preference instead.
  • [ ] IDE experience (formatting/NormalizeWhitespace, completion, highlighting/QuickInfo/FindAllReferences, UseImplicitType/UseExplicitType)

jcouv avatar Feb 02 '18 20:02 jcouv

Why would you discourage new() for primitive structs if you don't already discourage new int()?

jnm2 avatar Feb 02 '18 20:02 jnm2

@jnm2 I'm not convinced that it should be disallowed, but @gafter felt it adds no value. I'll let him elaborate if needed. You can write new Int32() but I've never seen it used. In the case of target-typed new, you could just write 0, which is simpler and clearer. On the other hand, it may not be worth worrying about. Blocking it could be irregular and more work.

jcouv avatar Feb 02 '18 20:02 jcouv

this feels like the "default should not be permitted for reference types because you can use null anyways" argument. I'm with others that it wouldn't worth it to restrict new() for primitive structs.

alrz avatar Feb 02 '18 20:02 alrz

On the other hand, it could cause overload resolution to fail on primitive types,

class C {}
void M(C c) {}
void M(int i) {}

M(new());

This won't compile if we don't restrict primitives, which may or may not be desirable - any restriction on permitted types would have the same effect on the overload resolution - raising the success rate. I'll mention this in the proposed spec for LDM review.

alrz avatar Feb 02 '18 21:02 alrz

One scenario we should have a test for: this conversion should not be considered for input to a user-defined conversion operator:

class Dog
{
    public Dog() {}
}
class Animal
{
    private Animal() {}
    public static implicit operator Animal(Dog dog) ...
}

class Program
{
    public static void M(Animal a) ...
    public static void Main()
    {
        M(new()); // should fail rather than construct a Dog and convert to Animal
    }
}

gafter avatar Feb 02 '18 22:02 gafter