dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.01k stars 4.03k forks source link

Allow void function to return anything #11330

Closed Thaina closed 7 years ago

Thaina commented 8 years ago

Many time I write the code that would check and return early. I want to return with one line if. But C# does not allow return void so I need to make an if block if I want to log or call some method

void DoSomething(string key)
{
    if(dictionary.ContainsKey(key))
    {
        Debug.Log(key + " exist");
        return;
    }

    // Do some loading work
}

I want to let C# allow this

void DoSomething(string key)
{
    if(dictionary.ContainsKey(key))
        return Debug.Log(key + " exist");

    // Do some loading work
}

This is normal in js and I think it make code more compact

dsaf commented 8 years ago

This is normal in js...

It wouldn't be a valid argument even if the language chosen for comparison wasn't developed in 10 days, not to mention dynamic and untyped.

GSPP commented 8 years ago

The goal of this feature request is very understandable but I think this would sacrifice too much safety against bugs. Return type mismatches are quite common during refactorings in my experience. It's good that tools consistently find them.

Thaina commented 8 years ago

@GSPP Your point also understandable but at least we could allow return void with void function (Debug.Log is also void function)

jmarolf commented 8 years ago

You can do this today.

void DoSomething(string key)
{
    if(dictionary.ContainsKey(key))
        Debug.Log(key + " exist"); return;

    // Do some loading work
}

We just don't allow return to have arbitrary expressions after them in a void returning method. The hope here is that someone looking at the code can clearly see when a method returns and what is being returned.

My reading of your proposal is that any expressions after a return statement be checked to see if they are void return invocation expressions. I presume you would only allow a single void returning invocation expression? If that is the case then that invocation is called before the method returns. Is this a correct interpretation?

Thaina commented 8 years ago

@jmarolf Are you sure that

if(dictionary.ContainsKey(key))
        Debug.Log(key + " exist"); return;

would not be translate to

if(dictionary.ContainsKey(key))
        Debug.Log(key + " exist");
return;

?

I think I got Unreachable code detected warning with your code

jmarolf commented 8 years ago

Ah yes you are correct! Lets pretend I wrote this

if(dictionary.ContainsKey(key)){
        Debug.Log(key + " exist"); return;}
orthoxerox commented 8 years ago

Theoretically, I wouldn't mind being able to return void from a void-returning function, e.g., if Debug.Log actually returned void. Returning anything at all sounds dangerous to me, since void is not a supertype of any other value other than never/bottom (which is what a throw expression will pretend to return).

If we ever get never/bottom-returning functions, then I wouldn't mind being able to return the result of their invocation from a void-returning function.

GSPP commented 8 years ago

I never understood why void was not a normal type with values. If that was the case the code would naturally work.

Surely, void only exists because C had it for now totally obsolete reasons?!

Has there ever been a proposal to make void into a normal type? Seems like an obvious candidate for improvement.

Thaina commented 8 years ago

@GSPP Problem is how can you pass void in parameter and how that parameter would behave in that function. You may not allow it but then there are generic, would you allow generic to accept void

If not then it has not enough purpose to allow it

I just want syntactic sugar

void DoSomething(string key)
{
    if(dictionary.ContainsKey(key))
        return Debug.Log(key + " exist");
}

actually translate to

void DoSomething(string key)
{
    if(dictionary.ContainsKey(key))
    {
        Debug.Log(key + " exist");
        return;
    }
}

Without any CLR changed

At first I think it actually translate like this so it could be return any type, it will just ignore. But if you all think it too dangerous then I'm ok to just return only void function

svick commented 8 years ago

@GSPP #234

GSPP commented 8 years ago

What's the problem with void F(void x)? It does not make sense but it could be well defined. You could even pass x along: void F(void x) { return F(x); }. Doesn't make sense but it's consistent. This can simplify generics, it would not complicate them.

Generating a void value should be an easy thing, maybe just F(void); or if that doesn't work for some reason this: F(default(void)); or F(new void()); or F(new System.Void());?

void would behave like struct VoidType {} which I have actually seen defined in some project. I think it was the BCL.

khionu commented 8 years ago

Void is to emptiness. Void is sort of a placeholder for a return type, in practice. When something goes "into a void", in philosophical terms, you don't expect anything back.

Now, it seems that you have a desire for something that just ends the function after calling a method.... that sounds far more reasonable as a new keyword, though the syntactic sugar value is rather low....

HaloFour commented 8 years ago

Sounds like a good use for guard blocks (#8181), although that would not reduce the verbosity but it would at least ensure that you don't accidentally forget the return:

void DoSomething(string key)
{
    guard (!dictionary.ContainsKey(key)) else
    {
        Debug.Log(key + " exist");
        return; // would be a compiler error to omit this
    }

    // Do some loading work
}
orthoxerox commented 8 years ago

@GSPP I don't think void should have a real value, since then we have to decide if it's a value or a reference type. Having its only value be null would be an elegant solution, but it conflicts with non-nullable reference types, unless void remains special.

bbarry commented 8 years ago

@orthoxerox void (the concept) could easily have a value (of default(Void)):

public struct Void {}

This is useful when manipulating expression trees and dealing with tasks like so:

public Task DoSomething()
{
    TaskCompletionSource<Void> tcs = new TaskCompletionSource<Void>();
        //...
    return tcs.Task;
}

In fact it is actually defined in mscorlib: https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Void.cs (attempts to use it in C# cause CS0673).

This said, the value of void is not a part of this issue.

DemiMarie commented 8 years ago

I would be happy allowing return expr, where expr is a void-valued expression. But not otherwise.

Thaina commented 8 years ago

I'm not sure I made it clear or not that what I propose is not I would let void function will return anything out of function. It just return anything to the void

For example

void A()
{
    return 0; // not error, but it really don't return anything, just return out of stack
}

void B()
{
     int i = A(); // Compile error, void function not return anything
}

What I think is because some function may return a code but we don't care that code just want to call function

Such as

int Main(string[] cmd)
{
    return 0;
}

bool already = false;
void PrepareToCallMain()
{
    if(already)
        return Main(["already"]); // just return to the void

    // prepare everything
    already = true;
}
aluanhaddad commented 8 years ago

This is normal in js and I think it make code more compact

Doing that in JavaScript is really scary because every function returns a value. What if debug.log were refactored for convenience to return its argument, like

var debug = { 
    log: function (x) { return x; } 
};

Then your consumers would actually get a truthy value from the method in the case where no work was performed. That would really be misleading. e.g.

if(loadUserData(userName)) { 
    console.log('loaded data for ' + userName + ' into the cache.'); // oops
}

Back in C# land, this seems like a refactoring hazard. I just don't see the value. Guard blocks are a good idea. Perhaps they could be modified to return implicitly from void methods. That would fit your usecase without being confusing. I'm thinking

void DoSomething(string key)
{
    guard (!dictionary.ContainsKey(key)) else
    {
        Debug.Log(key + " exist");
    }

    // Do some loading work
}
Thaina commented 8 years ago

@aluanhaddad Look at comment above you

aluanhaddad commented 8 years ago

I read the entire discussion, what are you insinuating?

Thaina commented 8 years ago

@aluanhaddad If you really read what I said you should see the word return to the void or it really don't return anything, just return out of stack

You still state that every function returns a value is evidence that you just randomly make argument without carefully read anything

aluanhaddad commented 8 years ago

@Thaina I was talking about JavaScript in reference to your original post where you use it as a point of comparison.

In JavaScript every function returns a value. I was going off on a tangent, and for that I appologize.

However, I clearly marked which language I was referring to.

user1568891 commented 8 years ago

I would like at least allow return voidMethod(); as a syntactic sugar statement inside other void methods.

dmartensson commented 7 years ago

Maybe use

void DoSomething(MyObject a) {
    if(a == null)
        return void Log.Debug("");
    ... do real work
}

where the void keyword indicates that we want to return and discard any return value from the called method.

It would visually indicate returning nothing while still avoiding extra indentation

void DoSomething(MyObject a) {
    if(a == null)
    {    
        Log.Debug("");
        return;
    }
    ... do real work
}

The point of using return void is that it both is a visual indication that we return without a value and also would allow us to use both void methods and methods returning a value in the return.

The compiler will never have to guess if we wanted to return the value or ignore it, it can still give a warning if we try to return a method that returns a value and we did not use the void keyword.

Void is in this case used as discards the value.

jnm2 commented 7 years ago

I like this proposal just for the ability to return a void expression in a void method. I think it brings a good symmetry.

uecasm commented 7 years ago

I'm reasonably sure that at the IL layer it's happy with return someVoidMethod(); it's just C# that doesn't permit it.

In a method with a void return type, I would like return someVoidMethod() to be valid; I don't like return someNonVoidMethod() as this is way too error-prone, but perhaps return (void) someNonVoidMethod() would be ok -- that's similar to syntax that works in C++ without warnings.

Of course, in a method with a non-void return type, it wouldn't be legal, since the types don't match.

Somewhat related, I've occasionally found myself writing very similar code for methods that take and return Task vs. Task<T>. Usually the only differences are in the return, yet you can't have a Task<void> (or actually a Task<T> where T is void) that would work for both. I suspect the ship has sailed on this one, however. (And it would require void to be a "real" type capable of being used as a variable type.)

Thaina commented 7 years ago

return (void)someNonVoidMethod() would be a good solution to avoid error

Also I think I have seen some issue to allow void as type (so it can put in generic). It is almost agreeable that it should. But they have no timeline about it and no one would like to go do it because there are so many more important or easier work that should be done before

jnm2 commented 7 years ago

Plus, Task<void> or Task<()> would never be the same as Task. You'd still have to write both implementations because there are two distinct classes involved.

uecasm commented 7 years ago

@jnm2 No, you're missing the point.

public async Task<T> DoSomethingAsync<T>(Func<Task<T>> action)
{
    // ... some prep work ...
    var result = await action();
    // ... some other work that doesn't care what "result" is ...
    return result;
}

If Task<void> were legal, then the above is the only implementation you'd need -- you don't need to provide implementation for Task at all (barring backwards-compatibility issues); the above is sufficient by itself to cover both void return and non-void return cases. Currently, however, implementing the above is not sufficient, you have to provide a separate overload for Task that omits use of result entirely as well in order to handle the void return case, even if everything else is identical. While you can factor some of this out to minimise duplication of code, sometimes it's awkward (eg. if there are using or foreach statements involved).

Doing this properly would require void to be the same as any other type, though, including allowing constructs like IEnumerable<void>. Although the JIT optimiser could be smart enough to elide that sort of thing. And that's a wider argument than this issue was originally making and is probably already discussed elsewhere.

jnm2 commented 7 years ago

barring backwards-compatibility issues

That's the point I was talking about. :smile: I don't think it's likely that we'll be able to deprecate Task; therefore, I imagine you'll need to continue accepting it as a parameter type.

uecasm commented 7 years ago

In framework and general-use library code, sure. In application and app-specific library code, perhaps not.

Though actually there might not be all that many compatibility issues with it. Task would still have a place as the base class of Task<T>, and Task<void> is a Task<T> so is also a Task. If the compiler were to quietly pretend that an async method with return type Task actually returned Task<void> (not that I'm necessarily advocating that), most code wouldn't notice any difference. (If both overloads exist, it would use the Task<T> overload instead of the Task one, which is a possible behavioural change; if only the Task overload exists then it would continue to use that with no change.)

(If you left methods alone and simply made it apply only to async lambdas, then even less code would notice.)

Still, you're right (and I said before) that it's one of those things that probably needs a time machine to solve nicely. The gang-of-four generic delegate API explosion (Action, Func<T>, Func<Task>, Func<Task<T>>) just irks me a bit, that's all. (Action == Func<void> is the other half of making void a real type that would cut this in half so you only have sync vs async.)

Thaina commented 7 years ago

@uecasm We already have some arguments in https://github.com/dotnet/csharplang/issues/135 about that issue. And the main reason I agree that making void be type to put in generic should not be done because it will make this

class A<T>
{
    public void Do(){}
    public void Do(T obj){}
    public void Do(T l,T r){}
}

be ambiguous

You can argue with other people there

uecasm commented 7 years ago

I don't see why that's ambiguous. Zero, one, or two parameters are still entirely distinct.

I suspect we have a philosophical difference here on what void would mean as a real type. In my view, it would be a unit type, ie. something that can have only one value (like a bool that can never be true or a reference that can only be null). Given that its value is always known and can be reconstructed on demand at any time, the compiler never needs to allocate storage for it, but it's still countable -- ie. an IEnumerable<void> can be a thing that exists and reports that it contains 3 items -- they're just always default(void) and could never be any other value.

So in the above, all three methods are distinct and overload resolution would happily select between them, but the JIT compiler would know it doesn't need to actually put the parameters on the stack since the method would never actually access them anyway. (If they tried, it could just use default(void).)

legistek commented 2 years ago

I found this proposal while looking for any similar open issues before proposing it for myself in https://github.com/dotnet/csharplang. I'm surprised it was not more well received. With the recent trend in C# to simplify and shorten the syntax, this one seems like a no-brainer, saving 3 lines of code (2 curly braces plus a return) for something that is very common.