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 "deprecate _ (underbar) as an identifier" #1064

Closed
5 tasks
gafter opened this issue Oct 31, 2017 · 87 comments
Closed
5 tasks

Champion "deprecate _ (underbar) as an identifier" #1064

gafter opened this issue Oct 31, 2017 · 87 comments

Comments

@gafter
Copy link
Member

gafter commented Oct 31, 2017

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

It is unfortunate that the reader cannot easily distinguish at the use site between a discard and a use of an identifier named _. Presumably the latter is rare. Programs would be easier to read if one could reliably depend on _ being a discard.

@jnm2
Copy link
Contributor

jnm2 commented Oct 31, 2017

Edit: I've changed my position on this: #4460 (comment)

I use .Select(_ => _.Foo) everywhere, and I want to continue doing so.

@HaloFour
Copy link
Contributor

Relevant previous discussions/arguments/rants:

dotnet/roslyn#14794
dotnet/roslyn#14862

There might be a few more.

I think it's worth the discussion although it would probably be difficult to resolve without breaking a lot of existing code, particular like @jnm's example.

🍝 Perhaps the rules for identifiers could allow underbar for a lambda parameter in expression lambdas but not elsewhere? That would be weird.

At the very least perhaps consider adding a warning wave to declaring a field or local using the underbar identifier. You could differentiate the discard/local case by whether or not the local is ever used.

@bondsbw
Copy link

bondsbw commented Oct 31, 2017

If there was an auto-upgrade conversion of existing usages of _ as an identifier to @_, it might not break so much existing code.

@jnm2
Copy link
Contributor

jnm2 commented Oct 31, 2017

The point of _ => _.Foo is that _ is the least possible visual clutter. I'm not a fan of @_ => @_.Foo.

@bondsbw
Copy link

bondsbw commented Oct 31, 2017

@jnm2 Agreed, it would just keep from breaking existing code.

@iam3yal
Copy link
Contributor

iam3yal commented Oct 31, 2017

@jnm2 Well, you can always use something like x, however, I understand why you would dislike it.

p.s. iirc @HaloFour made a proposal that addresses exactly this or something similar where we can do something like @.Foo or .Foo (can't remember) instead of _ => _.Foo.

@jnm2
Copy link
Contributor

jnm2 commented Oct 31, 2017

I'd stop using _ => _.Foo in a heartbeat if I could use @.Foo.

@iam3yal
Copy link
Contributor

iam3yal commented Oct 31, 2017

@jnm2 Indeed, it would be awesome if they would at one hand get rid of _ as a valid identifier and on another hand would allow people to do something like @.Foo.

@alrz
Copy link
Member

alrz commented Oct 31, 2017

I'd support any kind of deprecation at this point. This seemed impossible to me but now I can see the light. Please deprecate more.

@MgSam
Copy link

MgSam commented Oct 31, 2017

Is this seriously being considered or is this just a wish? Has the LDC suddenly had a change of heart where they're willing to take breaking changes?

@yaakov-h
Copy link
Member

What does "deprecate" mean in this context? I assume it wouldn't be a full removal from the language?

@iam3yal
Copy link
Contributor

iam3yal commented Oct 31, 2017

@yaakov-h I assume that in any place where today you can use a single _ as a valid identifier it wouldn't be possible with this change.

@Joe4evr
Copy link
Contributor

Joe4evr commented Oct 31, 2017

At the very least perhaps consider adding a warning wave to declaring a field or local using the underbar identifier.

I recall that something like this was already planned to be the case. And the whole process would likely be spread out over a few major language versions, regardless. (So for example, 8.0: Info, 9.0: Warning (maybe with exclusion from /warningsAsErrors), 10.0: Error. But that's just spit-balling.)

@kentcb
Copy link

kentcb commented Nov 1, 2017

Stumbled on this by accident and just had to add my 2c. I'd be very happy if _ was treated as discard consistently in C#. We're in a weird place right now where it's sometimes discard, sometimes not. I think that was a mistake, but one better fixed late than never.

As someone who makes heavy use of Rx, being able to discard with _ in lambdas would be 💯. Random example of one place this would be useful:

this.WhenAnyValue(
    x => x.DepartureLocation,
    x => x.ArrivalLocation,
    x => x.JourneyMode,
    x => x.JourneyTime,
    x => x.ShowAll,
    (_, __, ___, ____, _____) => Unit.Default)

Having said that, I understand concern from @jnm2 et al regarding breaking changes. Personally, I think _ (and x, and @) are all terrible names for lambda parameters when a more descriptive name adds value (not the case above because the x there is just a stand-in for this). For example, this is much better:

items.Select(item => ...)

But of course, now we're getting into style arguments...

@jnm2
Copy link
Contributor

jnm2 commented Nov 1, 2017

There's no reason we couldn't have this, and there's even a proposal on it:

this.WhenAnyValue(
    _ => _.DepartureLocation,
    _ => _.ArrivalLocation,
    _ => _.JourneyMode,
    _ => _.JourneyTime,
    _ => _.ShowAll,
    (_, _, _, _, _) => Unit.Default)

items.Select(item => item.Foo) does not seem more descriptive than items.Select(_ => _.Foo).

@kentcb
Copy link

kentcb commented Nov 1, 2017

There's no reason we couldn't have this, and there's even a proposal on it

Er, yes. It's what I'm asking for. But it would require that this proposal be accepted so that _ is not an identifier. Or maybe I'm missing something...?

items.Select(item => item.Foo) does not seem more descriptive than items.Select(_ => _.Foo)

It was a very simple example based on yours. Add a SelectMany and a Where, all the while calling all variables (of differing) types _, then see how easy it is to read :)

@jnm2
Copy link
Contributor

jnm2 commented Nov 1, 2017

There's no reason we couldn't have this

Er, yes. It's what I'm asking for.

The code sample uses _ for both parameter names and discard– it didn't sound like you were asking for that.

Add a SelectMany and a Where, all the while calling all variables (of differing) types _, then see how easy it is to read :)

Though in fact it's most predominantly used only once in a chain, I do this on a regular basis and like it:

foos.SelectMany(_ => _.Bars).Any(_ => _.Start != null || _.End != null)

The project I have open: 16.9% of the files use _., and in those files it occurs 6.59 times per file.

@kentcb
Copy link

kentcb commented Nov 1, 2017

The code sample uses _ for both parameter names and discard

Ah, I overlooked your use of _ for parameter names. That is certainly not what I'm asking for. Thanks for clarifying.

@DarthVella
Copy link

@eyalsk I'd assumed a compiler warning for a while, up until it is outright disallowed on the next major language update (or whenever).

@MkazemAkhgary
Copy link

Attention please

Why not make it an option to enable/disable usage of discards all over the place?

like how unsafe is optional. that way it wont be breaking change.

@DavidArno
Copy link

Whilst this is a breaking change, it is only breaking if one uses "treat warnings as errors" (which hopefully we all do; oh to have that as the default in new projects!) and it need only be breaking for as long as it takes to select a "ignore this warning in this project" code fix.

@iam3yal
Copy link
Contributor

iam3yal commented Nov 1, 2017

@DarthVella Yeah, probably. :)

@DavidArno
Copy link

@DarthVella, @eyalsk,

Warning in v8, error in v9 seems a nice timeline for phasing out the use of _ as an identifier. I'd certainly not like to see either introduced in a minor release.

@iam3yal
Copy link
Contributor

iam3yal commented Nov 1, 2017

@DavidArno Yeah this is what I wrote in a comment to @MkazemAkhgary in his post. ;)

@MkazemAkhgary
Copy link

for a very rare case, __ (two _) wouldn't be a warning. unless they decide to deprecate underbar-only identifiers.

(_, __) // former becomes discard, latter remain variable.

@DavidArno
Copy link

@MkazemAkhgary,

I'd personally like to see all underscore-only identifiers deprecated, even though I know this would break some of my own code (and thus the code of anyone who uses my library). The benefits of clarity would outweigh the small pain of fixing that broken code in my view.

@sharwell
Copy link
Member

sharwell commented Nov 1, 2017

@gafter My previous understanding was it's possible to automatically turn all _ identifiers in a scope into discards when there are two or more of them (a situation which previously resulted in a compiler error). This is a both a non-breaking change and delivers the "discards everywhere" feature. To me, the approach seems like an obvious choice; is there a reason why it didn't work in practice?

📝 With this behavior in place, an analyzer could be used to deprecate _ as an identifier in code bases - specifically to catch cases where a single _ is not being used as a discard.

📝 I have some code bases which this change would break. It wasn't commonly used, but I'm loathe to change the affected cases because they are in some of the more stable parts of code I've worked on in the past (approaching zero churn).

@aluanhaddad
Copy link

aluanhaddad commented Nov 5, 2017

@gafter thank you for the clarification.

@jnm2

Wait... are you saying @gafter doesn't believe in free will after all... 😁

You're making me feel philosophical ♥

@DavidArno I disagree about deprecating var as an identifier.

@thomasd3
Copy link

thomasd3 commented Nov 10, 2017

Visually, _ is good because it 'aerates' the display and I use _ extensively as (_ =. _.Data).

If it gets deprecated, then I would also agree that (@.Data) would work; But don't deprecate something until there is a better replacement.

@4creators
Copy link

There is an old saying in .NET API world: once something is released you are stuck with it for lifetime. Reason: do not break existing code. The same applies to C# language requiring guarantee for backward compatibility.

IMO weather someone likes it or not "_" - underbar is available as an identifier and it should remain to be available in the future.

Perhaps it would be better to extend design and prototyping stage for C# features to avoid that kind of issues in future rather than messing with existing code.

@GeirGrusom
Copy link

@4creators

Perhaps it would be better to extend design and prototyping stage for C# features to avoid that kind of issues in future rather than messing with existing code.

Absolutely what they normally do, but this was a bikeshedding issue.

@Pzixel
Copy link

Pzixel commented Nov 15, 2017

@4creators and what would you do currently in this situation when all other characters are in use? Probably we could use some Emoji instead?

It's good when language is backward-compatible, but sometimes it limits you without giving worthwhile profit. You can't develop language in centuries and still backward compatible. So I'd like C# to evolve and loose some of these features like underscore identifier rather than see it slowly dying because of focusing on backward compatibility too much. Especially when we have wonderful @.SomeProperty syntax that fix, I guess, 80% of all issues. What leaves? Hmm, there was an example above:

public static IEnumerable<T> Pipe<T>(this IEnumerable<T> source, Action<T> action)
{
    if (source == null) throw new ArgumentNullException(nameof(source));
    if (action == null) throw new ArgumentNullException(nameof(action));

    return _(); IEnumerable<T> _()
    {
        foreach (var element in source)
        {
            action(element);
            yield return element;
        }
    }
}

I spend, don't know, half of minute until I realized what's happening here. It's weird, it's confusing and spending so much time for that simple code is not ok. So I would happy if this doesn't compile.

@4creators
Copy link

4creators commented Nov 15, 2017

@Pzixel

It's good when language is backward-compatible, but sometimes it limits you without giving worthwhile profit. You can't develop language in centuries and still backward compatible.

Evolving language does not have to loose backward compatibility. Just look at how C++ has evolved over last 10 years and it allows for a lot of bad, and hard to understand code and at the same time you may find quite a lot of beautifully written code. It is the users of language who make it easy or hard to understand usually the same goes true for natural languages as well.

@MkazemAkhgary
Copy link

I spend, don't know, half of minute until I realized what's happening here. It's weird, it's confusing and spending so much time for that simple code is not ok. So I would happy if this doesn't compile.

that's unfair argument, you can always write hard to understand code, you can always abuse options you have in hand.

@Pzixel
Copy link

Pzixel commented Nov 16, 2017

Evolving language does not have to loose backward compatibility. Just look at how C++

You know, this example doesn't help you as well as it helps me. C++ become almost unsupportable because of this and sometimes it's better and easier to understand code on another language, e.g. Rust, than other C++ guy's code.

that's unfair argument, you can always write hard to understand code, you can always abuse options you have in hand.

I understood this example as a sample of underscore-identifier feature fair use, not abuse.

@DavidArno
Copy link

DavidArno commented Nov 16, 2017

@4creators,

Evolving language does not have to loose backward compatibility. Just look at how C++ has evolved over last 10 years and it allows for a lot of bad, and hard to understand code...

You are right; a language doesn't have to sacrifice backward compatibility to evolve. But your example of C++ proves why always keeping backward compatibility leads to problems. Sacrifice backward compatibility in a few cases and the language can evolve cleanly, not end up like a dog's dinner like C++.

Breaking backward compatibility just for the sake of it is not good. But occasionally, breaking it really helps. Getting rid of underscore-only identifiers would really help.

@MkazemAkhgary
Copy link

MkazemAkhgary commented Nov 16, 2017

I think _.Foo is cleaner than @.Foo... I don't know how to describe it, so ill give example. you probably get headache if you try to read Chinese writing because of all of that crooked shapes.

_ is way less crooked than @. it would be nice if they deprecate _ identifier and make is supersede of @ #91

@Pzixel
Copy link

Pzixel commented Nov 16, 2017

@MkazemAkhgary you don't choose between _.Foo and @.Foo, but rather between @.Foo and _ => _.Foo. I find @ much clearer here.

@MkazemAkhgary
Copy link

@Pzixel if _ becomes deprecated then _.Foo will be a valid syntax too. so between _.Foo and @.Foo I find _.Foo less noisy and cleaner.

I don't see how _.Foo is not valid in that case?

@iam3yal
Copy link
Contributor

iam3yal commented Nov 17, 2017

@MkazemAkhgary Well, if they will deprecate it then it will probably take some time before it's going to be completely disallowed whereas @.Foo can be done today.

@Pzixel
Copy link

Pzixel commented Nov 17, 2017

@MkazemAkhgary if _ is lambda parameter then it's better to use @ and to not write arrows and so on at all. It it's a variable, please, don't name it so.

Hey, Java 9 has also deprecated underscore. It seems that this restriction actually has a real value.

@MkazemAkhgary
Copy link

MkazemAkhgary commented Nov 17, 2017

there are 3 ways I can think of, but I think only one of them should be eligible.

  1. _ remains as identifier, new shorthand lambda syntax will be src.Select(@.Foo), old syntax like src.Select(_ => _.Foo()) will be still valid.

  2. soon src.Select(@.Foo) becomes new lambda syntax and after that _ becomes deprecated with only one usage i.e discards.

  3. _ becomes deprecated in future, after that src.Select(_.Foo) becomes new lambda syntax. so _ has both discards usage and shorthand lambda usage.

In my opinion, option 3 is best. don't rush on things, wait for language to slowly become evolved but in much cleaner form and shape.

@DavidArno
Copy link

DavidArno commented Nov 17, 2017

@MkazemAkhgary,

Thanks for the three point description, as I was getting really confused by the talk of _.Foo.

However, I have to say that I think option 3 is a truly awful idea. Removing _ as an identifier, to then make it both a discard and a lambda shortcut just makes no sense to me. The only option that I'd favour, is option 2. It gets rid of _ as an identifier; it makes _ only a discard; and it uses @ as a lambda shortcut. That's a triple win in my view.

@orthoxerox
Copy link

On the other hand, it looks like F# might get _.Foo as a lambda shortcut (albeit only in this specific case): fsharp/fslang-suggestions#506

@Vlad-Stryapko
Copy link

Vlad-Stryapko commented Nov 17, 2017

@DavidArno

both a discard and a lambda shortcut just makes no sense to me.

Once again bringing up Scala's design, that's exactly what they have and it's fine most of the time, though it does cause some confusion for newcomers in certain scenarios with lambdas.

Overall, I still think option 3 is the best one to have. We're a little bit in subjective syntax ground here, but for me _ looks way cleaner than @.

@GeirGrusom
Copy link

Kotlin uses It

val user = users.filter(It.name == "Foo").singleOrNull();

@jnm2
Copy link
Contributor

jnm2 commented Nov 24, 2017

Just ran across this:

fixed (char* _ = str)
{
    // ...
}

The pointer is not used, so it's a more efficient equivalent to this:

var pinningHandle = GCHandle.Alloc(str, GCHandleType.Pinned);
try
{
    // ...
}
finally
{
    pinningHandle.Free();
}

@ErdoganKurtur
Copy link

I have a jQuery-like code library. instead of $ it uses _. Now you will break my library and all the codebase that references it just because you do not like it? This is another spaces vs tabs.

I believe that running analysis on github repositories for detection of use cases for _ is a good intention, but all the code in the world is not on github.

@DavidArno
Copy link

DavidArno commented Dec 5, 2017

@ErdoganKurtur,

Now you will break my library and all the codebase that references it just because you do not like it? This is another spaces vs tabs.

This is not the case. The proposed change isn't "just because we do not like it". It is because _ is the discard symbol, but it is also currently valid as an identifier. This causes confusion, and - assuming your _ is a global identifier (" jQuery-like code library" suggests it is, but they may be a false assumption on my part) - if I used your library, it would prevent me using _ as a discard as the identifier takes precedence.

Thus the proposal: remove _ as an identifier and only make it a discard, to make life simpler for everyone in the long run.

@IS4Code
Copy link

IS4Code commented Jan 4, 2018

This should have been thought about when the discard _ was first introduced. With the already present mishmash of patterns and paradigms, this is a minor inconsistency in C#.

@gafter
Copy link
Member Author

gafter commented Jan 26, 2018

Pattern-matching will not permit you to reference a constant named _ as a constant pattern. It will be interpreted as a discard. Given that, I withdraw this proposal.

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