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

modifier return values #49

Closed
sillytuna opened this issue Sep 7, 2015 · 22 comments
Closed

modifier return values #49

sillytuna opened this issue Sep 7, 2015 · 22 comments
Labels
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

@sillytuna
Copy link

Request for modifiers to be able return a value. Specifically, this allows modifiers to be used on functions which have return codes.

@nmushegian
Copy link

I see two interpretations here, one is much simpler to implement but is less powerful:

  1. Modifiers can have a return type, but then a modifier must have the same return type as any function that uses it and the function must be external (this is easier, just emit RET where there is a return in the modifier)

  2. Allow modifiers to have any return type, and somehow extend "_" to capture a return value

@chriseth
Copy link
Contributor

chriseth commented Sep 10, 2015

I am also not sure how to do this properly. Please note that modifiers are very similar to macros and different from functions calling functions, especially with regards to return:
If there is a return in the function, the part after the _ is not run. If the function assigns to the return variables (but has no explicit return and just "runs off" the end of the function), the part after the _ is run.
So one solution to implement something like this would be to bind the return variables to variables in the modifier (similar to how the values of input variables can be forwarded to the input variables of the modifier), but I have no good idea as to how to do this syntactically.

@nmushegian
Copy link

What about just option 1 then? A lot of the time we use modifiers, we are implicitly using them as if they are attached to a function with a bool-like type which has two options: 0 or everything else (because modifiers "return 0" when they don't execute the _). So it'd be nice to explicitly say "this modifier is for bool-returning functions" except for all fixed-size types, then let us actually return stuff.

@nmushegian
Copy link

Might make sense to make it a distinct language feature if it also imposes the constraint that the function has to be external. It might be deviating too far from what a modifier is supposed to be.

@chriseth
Copy link
Contributor

Yes, option 1 sounds good. To summarize again:
If a modifier has a returns specifier, the modifier can only be applied to a function with exactly the same return signature. The return variables in the modifier are bound to the return variables of the function.
If it does not have a returns specifier, it can be applied to any function and is not able to modify the return variables.

@sillytuna
Copy link
Author

Option 1 works - a returns (...) format which must match the signature of any functions it's used on. Thanks for looking at this.

@axic
Copy link
Member

axic commented May 10, 2016

It would be nice to solve point 2 as mentioned by @nmushegian. Maybe a simple solution would be not allowing modifiers to change the return value, but to capture and return it so that the modifier body after the _ keyword could be executed even if the function returns.

i.e.

modifier {
  /* do stuff */

  var ret = _

  /* do stuff */

  return ret
}

Where ret would be behave like a reference to the memory or like a tuple? @chriseth is that something remotely feasible?

As a future progression, since you have the values as a variable, modifying/changing it could be introduced.

@chriseth
Copy link
Contributor

chriseth commented Aug 2, 2016

@axic I don't see where your solution would be needed. If we change the return to not skip modifier suffixes (and we have to do it for your solution), it would already work as is.

@nmushegian
Copy link

@chriseth is _ is already an expression? I don't think you can do var x = _, I don't see how that behavior is enough to address @axic's use case

@chriseth
Copy link
Contributor

chriseth commented Aug 5, 2016

@nmushegian if the modifier cannot return the return value, there is no need for capturing it. The modifier can just ignore the return value and it will be returned.

@chriseth chriseth added this to the 99-nice-but-how milestone Aug 5, 2016
@nmushegian
Copy link

I think I understand now, but I find it really unintuitive and still wish there was a way where return !_; could work. Oh well

modifier toggles() returns (bool result) { 
    _
    result = !result;
}
function getFalseByToggle() toggles returns (bool result) {
     return true;
}

@axic axic added feature and removed enhancement labels Sep 14, 2017
@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Jul 28, 2018
@bookmoons
Copy link

Instead of this:

function multiplierOf(address _account)
public
returns (uint8 multiplier) {
  if (accountTier3(_account)) return TIER_3_MULTIPLIER;
  else if (accountTier2(_account)) return TIER_2_MULTIPLIER;
  else return TIER_1_MULTIPLIER;
}

I'd like to do this:

function multiplierOf(address _account)
public
grantTier3(_account)
grantTier2(_account)
returns (uint8 multiplier) {
  return TIER_1_MULTIPLIER;
}

modifier grantTier3(address _account)
returns (uint8 multiplier) {
  if (accountTier3(_account)) return TIER_3_MULTIPLIER;
  _;
}

modifier grantTier2(address _account)
returns (uint8 multiplier) {
  if (accountTier2(_account)) return TIER_2_MULTIPLIER;
  _;
}

It keeps branches out of the body as recommended and splits out the checks for independent validation.

axic pushed a commit that referenced this issue Nov 20, 2018
EIP-8: devp2p Forward Compatibility Requirements for Homestead
@fulldecent
Copy link
Contributor

As somebody that audits Solidity code I would recommend the contract author to use a different approach to increase readability. I do not see a good use for this feature.

@martasaparicio
Copy link

Hi there. I'm writing a Smart Contract that follows the StateMachine and AccessRestriction common patterns of Solidity for my thesis.
I have this code:
function nextStage() internal { stage = Stages(uint(stage) + 1); } modifier transitionNext() { _; nextStage(); } function rq_rentalCompleting() transitionNext { if(true){return true;} else {return false;} }
So I would like to know if i can in same way get the retruned value of rq_rentalCompleting on the modifier transitionNext so i can change his behaviour accordingly on the nextStage funstion?

Thank you :)

@fulldecent
Copy link
Contributor

Stages(uint(stage) + 1) is entirely unsemantic.

Please use explicit enums and refer to those named enums in code, like all other state machines do.

@martasaparicio
Copy link

martasaparicio commented Jul 17, 2020

i will do that thanks!! mean while do you have any suggestion on how to receive a return value from a function into a modifier @fulldecent ?

@fulldecent
Copy link
Contributor

My answer will be application-specific. Please ping me from that project and let's discuss there.

@howardpen9
Copy link

i will do that thanks!! mean while do you have any suggestion on how to receive a return value from a function into a modifier @fulldecent ?

++

@fulldecent
Copy link
Contributor

I have answers to all these questions. But here is not a forum.

Please close this issue, there is no specific need to have a modifier return type. Here is what demonstration of a specific need looks like #3419 And I categorically assert that any Solidity program written with a modifier using return values can be written better without that feature.

You are welcome to make a full question on a questions website, including code you wrote and your own research, and you can even ping me @fulldecent. I'm on all the sites.

@pcaversaccio
Copy link

FWIW, I made a Twitter poll here:
image

@NunoFilipeSantos NunoFilipeSantos removed this from the 99-nice-but-how milestone Apr 6, 2023
@github-actions
Copy link

github-actions bot commented Jul 6, 2023

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jul 6, 2023
@github-actions
Copy link

Hi everyone! This issue has been automatically 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 Jul 14, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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