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

Champion "Lambda discard parameters" #111

Open
gafter opened this issue Feb 15, 2017 · 91 comments
Open

Champion "Lambda discard parameters" #111

gafter opened this issue Feb 15, 2017 · 91 comments

Comments

@gafter
Copy link
Member

@gafter gafter commented Feb 15, 2017

Examples: (_, _) => 1, (int _, string _) => 1, void local(int _, int _) ...

See

@gafter gafter self-assigned this Feb 15, 2017
@gafter gafter changed the title Discard for lambda parameters (_, _) => Champion Discard for lambda parameters (_, _) => Feb 15, 2017
@DavidArno

This comment has been minimized.

Copy link

@DavidArno DavidArno commented Feb 20, 2017

Please, please, please could this apply to methods too? Sometimes, the parameters must be there, but just aren't used. By way of example, this Unit type has to have both R# and CA1801 suppressions for the operator overloads:

public static bool operator ==(Unit u1, Unit u2) => true;
public static bool operator !=(Unit u1, Unit u2) => false;

Being able to write those with discards to reassure those checks that the parameters must be there, but aren't used, would clean up such code nicely:

public static bool operator ==(Unit _, Unit _) => true;
public static bool operator !=(Unit _, Unit _) => false;
@jnm2

This comment has been minimized.

Copy link
Contributor

@jnm2 jnm2 commented Feb 20, 2017

@DavidArno I believe the .NET Framework design guidelines have always been to use the names left and right specifically. https://msdn.microsoft.com/en-us/library/ms229004.aspx

@DavidArno

This comment has been minimized.

Copy link

@DavidArno DavidArno commented Feb 20, 2017

@jnm2,

Oh, I didn't know that. I'll give that a try to see if it makes the compiler happier. Would still like to be able to use discards though.

@gafter gafter changed the title Champion Discard for lambda parameters (_, _) => Champion "Discard for lambda parameters (_, _) =>" Feb 21, 2017
@acple

This comment has been minimized.

Copy link

@acple acple commented Feb 22, 2017

this proposal also permit in nested lambda expression _ => _ => true ?

Enumerable.Range(0, 3)
    .SelectMany(_ => Enumerable.Range(0, 3)
        .SelectMany(_ => Enumerable.Range(0, 3))); // i don't need parameters in nested
@gafter

This comment has been minimized.

Copy link
Member Author

@gafter gafter commented Feb 22, 2017

@acple Possibly not, unless we also do something like dotnet/roslyn#15793 at the same time.

@acple

This comment has been minimized.

Copy link

@acple acple commented Feb 22, 2017

@gafter Thank you. in my opinion, lambda parameter shadowing is a bit dangerous but, discarded parameters are safe to overcover. so that should support only with discarded parameters. is this shadowing issue? or discarding issue?

@gafter

This comment has been minimized.

Copy link
Member Author

@gafter gafter commented Feb 22, 2017

@acple This is a shadowing issue. When there is only one lambda parameter named _, it is a normal identifier that declares a parameter. It must be so for backward compatibility.

@acple

This comment has been minimized.

Copy link

@acple acple commented Feb 23, 2017

@gafter okay I understand. desire the identifier definition rules will be relaxed partially... thanks for the information! 😄

@MovGP0

This comment has been minimized.

Copy link

@MovGP0 MovGP0 commented Feb 27, 2017

there should also be a compiler warning/error when using an _ named variable:

items.ForEach(_ => Console.WriteLine(_));
@DavidArno

This comment has been minimized.

Copy link

@DavidArno DavidArno commented Feb 27, 2017

@MovGP0,

Unfortunately, that would be a breaking change. An analyzer could be added to this affect though.

@jnm2

This comment has been minimized.

Copy link
Contributor

@jnm2 jnm2 commented Feb 27, 2017

I think use of a single _-named parameter should be encouraged! I don't intend to use it any less.

@DavidArno

This comment has been minimized.

Copy link

@DavidArno DavidArno commented Feb 27, 2017

@jnm2,

I disagree. If I was going to use the parameter in the lambda body, then I'd use eg x. I think _ should only be used when the parameter(s) aren't used in the body, ie as discards.

I appreciate that's personal opinion though.

@jnm2

This comment has been minimized.

Copy link
Contributor

@jnm2 jnm2 commented Feb 27, 2017

@DavidArno Exactly, it is personal opinion. My rationale is that I'd like to not have to specify a parameter at all, so until I can do .Where(@.Foo != null) or similar, _ => _.Foo seems to be the nearest I can get to not specifying a parameter name, readability-wise.

@MovGP0

This comment has been minimized.

Copy link

@MovGP0 MovGP0 commented Feb 27, 2017

I disagree that it is a personal opinion, since it is a common convention in functional languages that _ is a 'throwaway' value that is not to be used. It confuses developers who are used to the convention.

Thankfully it is quite simple to give it a proper name.

@gafter

This comment has been minimized.

Copy link
Member Author

@gafter gafter commented Feb 28, 2017

I disagree that it is a personal opinion...

Oh, the irony.

@firelizzard18

This comment has been minimized.

Copy link

@firelizzard18 firelizzard18 commented Jan 11, 2018

Nested discards (_ => _ => <expr>) should be supported on at least an opt-in basis.

@OJacot-Descombes

This comment has been minimized.

Copy link

@OJacot-Descombes OJacot-Descombes commented May 2, 2018

Since a underscore is a valid identifier, shouldn't we use another character as discard character? I suggest the minus sign.

(-,-) => or - =>

@wanton7

This comment has been minimized.

Copy link

@wanton7 wanton7 commented May 2, 2018

If I remember correctly also nameof was a valid indentifier before it was added. For consistency _ should be used every for discards. When this is added I would expect a compile error if _ is accessed when C# target language version is same or greater than version where this was added.

@dadhi

This comment has been minimized.

Copy link

@dadhi dadhi commented Feb 14, 2019

Anyone knows if exist a similar proposal for Linq expressions from bindings?

@jeroen-mostert

This comment has been minimized.

Copy link

@jeroen-mostert jeroen-mostert commented Mar 20, 2019

I like this not just for itself but also so code analyzers can meaningfully start to issue "lambda parameter unused" warnings about code like

this.Bar = collection.Sum(c => Bar);

where of course the intent was to write

this.Bar = collection.Sum(c => c.Bar);

(And yes, I came here from fixing just such a bug.) CA1801 or vanilla Roslyn don't complain about this, and indeed such a warning would not be very practical today since it'd have to be suppressed very often. (I don't know if Resharper can warn for it, I don't have that. Having discards available makes the warning more viable regardless of the analyzer.)

@yaakov-h

This comment has been minimized.

Copy link
Contributor

@yaakov-h yaakov-h commented Mar 20, 2019

@jeroen-mostert A single-parameter lambda couldn't become a discard for backwards compatibility. This proposal only treats multiple parameters named _ as discards.

@jeroen-mostert

This comment has been minimized.

Copy link

@jeroen-mostert jeroen-mostert commented Mar 20, 2019

@yaakov-h: Understood, but if _ becomes idiomatic as a discard for all lambda parameters, then _ can be treated as a logical discard by analyzers even for single-parameter lambdas, leaving only the (unusual and exceptional) case of _ actually being used in the lambda body. (Which, realistically, analyzers can then also warn about as something you probably want to write differently.)

@gafter gafter moved this from 8.x Candidate to Any Time in Language Version Planning Sep 16, 2019
@gafter gafter added this to the Any Time milestone Sep 16, 2019
@jcouv jcouv added this to Implemented in Tracking: Julien Sep 20, 2019
@jcouv jcouv mentioned this issue Sep 24, 2019
3 of 34 tasks complete
@jcouv jcouv changed the title Champion "Discard for lambda parameters (_, _) =>" Champion "Discard parameters" Oct 25, 2019
@jcouv

This comment has been minimized.

Copy link
Member

@jcouv jcouv commented Oct 25, 2019

I renamed the issue as it is being broadened from lambda parameters to any/all parameters.

Note that we'd like to avoid variables being named _ (underscore), so we're tracking an analyzer or warning (in warning wave) to avoid any ambiguities (dotnet/roslyn#26594).

@gafter gafter removed their assignment Oct 27, 2019
@jcouv jcouv changed the title Champion "Discard parameters" Champion "Lambda discard parameters" Oct 28, 2019
@jcouv

This comment has been minimized.

Copy link
Member

@jcouv jcouv commented Oct 28, 2019

From discussion today in LDM, we're going to keep the feature narrow for now (only support it for lambdas and anonymous methods as initially proposed).

@YairHalberstadt

This comment has been minimized.

Copy link
Contributor

@YairHalberstadt YairHalberstadt commented Oct 28, 2019

Can I ask why doing this for all methods is more complicated?

@jcouv

This comment has been minimized.

Copy link
Member

@jcouv jcouv commented Oct 28, 2019

@YairHalberstadt It's not a question of implementation cost, but trade-off between language complexity and benefit to users. Methods are part of an API (they can be invoked, whereas lambdas can only be converted to delegates, so the parameters names are never accessible to callers).
It is possible that discards would be generalized to methods later on, depending on feedback. More notes will be posted for today's meeting.

@atifaziz

This comment has been minimized.

Copy link

@atifaziz atifaziz commented Oct 28, 2019

lambdas can only be converted to delegates, so the parameters names are never accessible to callers

Aren't they available through reflection? I rely on that in a library.

@jnm2

This comment has been minimized.

Copy link
Contributor

@jnm2 jnm2 commented Oct 29, 2019

@jcouv Can local functions be included? They are often converted to delegates.

@DavidArno

This comment has been minimized.

Copy link

@DavidArno DavidArno commented Oct 29, 2019

@jcouv,

You raise a good point re methods that I hadn't thought of. Consider the following code example,

public interface I 
{
    public void M(int a, int b);
}

public class C : I
{
    public void M(int _, int _) {}
}

public static class P
{
    private static void Main()
    {
        I i = new C();
        C c = new C();
        
        i.M(a:1, b:1);
        c.M(_:1, _:1);
    }
}

In C, I've used discards for M as I don't use the parameters. This makes sense to me: why name a parameter that isn't used?

But if I'm a developer working in a team that insists on named parameters (not sure if such a team exists, but thinking worst case here), then that line c.M(_:1, _:1); can't compile as I've trying to name those discards.

My motivation for wanting discards in methods (and operators, though I'm just lumping them in with methods here) is for situations such as when defining event handlers, operators and the like. In these situations, I have to specify the parameters to meet the contract. But if I then don't use those parameters, the compiler IDE complains with an IDE0060 message. So I want to use discards there to keep the compiler IDE happy.

But maybe this is the wrong solution to the problem. If a method must have a parameter to meet the contract, but that parameter isn't used, the compiler IDE puts the developer in the impossible situation difficult position of having to ignore that message. So maybe the simpler solution is to just not warn on those unused parameters when they are mandatory?

@Joe4evr

This comment has been minimized.

Copy link

@Joe4evr Joe4evr commented Oct 29, 2019

@DavidArno I think in the case you present, that should likely be a compiler error since C.M is an implicit implementation of I.M and there need to be unambiguous identifiers for the first and second parameter because M is a public API regardless of whether or not C happens to be followed by : I.

But, I think explicit implementations maybe could allow discarded parameters since those methods are only callable through the interface signature. A potential (but perhaps unlikely) addition to that would be to also allow it in method overrides provided that the overriding class is less visible than the base class? Idk, it's obviously quite a complicated scenario.

@HaloFour

This comment has been minimized.

Copy link
Contributor

@HaloFour HaloFour commented Oct 29, 2019

@DavidArno

So maybe the simpler solution is to just not warn on those unused parameters?

I agree, the better approach would seem to have the analyzers not generate diagnostics for unused parameters that are required to meet the contract, such as for implicit implementation of an interface member or for overloaded operators, etc.

In general I'm a little squeamish about extending _ to all parameters. Lambdas and local functions are at least internal implementation details, but public methods are more likely to be walked via reflection and I have concerns over how such parameters would be interpreted by tooling.

@Logerfo

This comment has been minimized.

Copy link
Contributor

@Logerfo Logerfo commented Oct 29, 2019

Including local functions sounds perfect. Even private methods have their reflection concerns.

@jcouv jcouv modified the milestones: Any Time, 9.0 candidate Dec 16, 2019
@jcouv jcouv moved this from Any Time to 9.0 Candidate in Language Version Planning Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.