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

Consider removing "named returns" #1401

Closed
ethers opened this issue Nov 20, 2016 · 6 comments
Closed

Consider removing "named returns" #1401

ethers opened this issue Nov 20, 2016 · 6 comments
Labels
breaking change ⚠️ closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.

Comments

@ethers
Copy link
Member

ethers commented Nov 20, 2016

Related to #719, #973, #708. Grouping the examples here.

These behaviours can be confusing for people who are starting out with the language. Solidity has been criticized as a language that hasn't been designed with safety in mind, and this is one feature where safety was overlooked imo.

contract Example {
    uint n = 2;

    function test() constant returns (uint n) {
        return n; // Will return 0
    }
}

And another:

contract Example {
    uint n = 2;

    function test() constant returns (uint n) {
        n = 1;
        return n; // Will return 1
    }
}

One more:

contract Example {
    uint n = 2;
    uint x = 3;

    function test() constant returns (uint x) {
        uint n = 4;
        return n+x; // Will return 4
    }
}

A case where the explicit return statement is "overshadowed" by the named return.

function foo() returns (uint r) {
    r = 1;
    if (somethingComplexThatReturnsFalse()) {
        return 2;
    }
}

Instead of somethingComplexThatReturnsFalse imagine a bunch of other code and loops.

@ethernomad
Copy link
Contributor

I've always really liked the named returns feature of Solidity. If anything return should be changed to not be able to take a value.

@redsquirrel
Copy link
Contributor

I would love to see us drop named returns altogether. I think it improves code readability to be explicit about returns.

@chriseth
Copy link
Contributor

We already have #973 (comment) for the first three examples. I think named returns in general are a great idea for readability - it will tell the user the meaning of the return value.

  • If you are talking about being able to assign to the return value, this is a different topic.
  • If you are talking about being able to use a non-explicit return in a function returning a value, it is also a different topic.

@ethers
Copy link
Member Author

ethers commented Dec 2, 2016

For readability, can the meaning of the return value be expressed as a code comment?

"being able to assign to the return value" is that this example?

function foo() returns (uint r) {
    r = 1;
    if (somethingComplexThatReturnsFalse()) {
        return 2;
    }
}

Or is the example a case of "non-explicit return in a function returning a value"?

Here's another "bad" one (0 will be returned):

function foo() returns (uint r) {
    if (somethingComplexThatReturnsTrue()) {
        return;
    }
    r = 1;
}

@ethers
Copy link
Member Author

ethers commented Jul 25, 2017

The "named" returns feature is another quirky one that gets burdened on Solidity writers and reviewers to get correct.

contract C {
    address[] public authorities;

    function foo() constant returns(uint count) {
        count = 2;
        return authorities.length;  // "Overrides" the "named" return. returns 0, the default for all EVM values
    }

}
contract C {
    address[] public authorities;

    function foo() constant returns(uint count) {
       return;  // return 0. It can appear that the default value (of 0) is returned.
    }

}
contract C {
    address[] public authorities;

    function foo() constant returns(uint count) {
        count = 2;
        return;  // returns 2. Here return doesn't return the default value (of 0) but acts like a "break".
    }

}

@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Jul 28, 2018
@NunoFilipeSantos NunoFilipeSantos added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 25, 2022
@github-actions
Copy link

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Jan 27, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

6 participants