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

add proposal for optional option-result #24

Merged
merged 1 commit into from Jun 27, 2022
Merged

add proposal for optional option-result #24

merged 1 commit into from Jun 27, 2022

Conversation

thomas-mangin
Copy link
Contributor

@thomas-mangin thomas-mangin commented Jun 25, 2022

This proposal suggests a way which could allow the language to let programmers return option-result without requiring their handling by the caller.

@medvednikov
Copy link
Member

Thanks for a nice RFC.

I agree with you, handling memory allocation should be allowed in V. Not by default though, to keep it clean. Kind of like how we do it with maps:

m[bad_key] // will panic
m[bad_key] or { ... } // handle 

@thomas-mangin
Copy link
Contributor Author

Thank you for the positive feedback. I will let you ponder what you want to do when you have the time to consider it. I wish you good continuation.

@vincenzopalazzo
Copy link

I agree with you, handling memory allocation should be allowed in V. Not by default though, to keep it clean. Kind of like how we do it with maps:

If we make this option, we could think to add a new build-in type instead to add a new syntax symbol like ??, and this builtin type can be unwrapped with or { }

@medvednikov medvednikov merged commit 542db09 into vlang:master Jun 27, 2022
@spytheman
Copy link
Member

I do not understand this RFC, sorry.

@spytheman
Copy link
Member

spytheman commented Jun 29, 2022

A way to achieve this goal would be to return option/result without requiring the caller to handle failure scenarios.

How does that solve the stated problem?

V aims to provide developers the tools required to write solid applications however panicing on memory allocation can leave the application and its data in a invalid state.

The developers should be able to handle these border cases without causing undue complexity on simpler applications.

@igotfr
Copy link

igotfr commented Jun 29, 2022

I do not understand this RFC, sorry.

I think that he want Option and Result like Rust

Many developers are happy with the state of things: memory allocation failures are rare and a panic is acceptable.
However the language could (and probably should) provide a way to handle these issues.

A way to achieve this goal would be to return option/result without requiring the caller to handle failure scenarios.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V already has a universal mechanism for that - namely not writing panic( ... ) but return error( ... ) instead. At the caller site you write only my_mem_allocating_fn()! (notice the ! at the end - as of now it is not clear whether ! will become ? eventually).

If there is any issue with suffix ! (or ?), then it is a bug.

But if there is not, then what is the motivation for this RFC?


```

The code could fail as the underlying `malloc` code could fail as array is calling malloc which can panic
Copy link

@dumblob dumblob Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a problem - but easily solved with ! as stated above. For convenience reasons V does not enforce ! nor or{ ... } at the call site at such scenarios (e.g. look at mymap[] which is also unsafe but you can make it safe by mymap[...]or{5} or mymap[...]!).

I would rather advocated introduction of a -strict/-paranoid/-nopanic/-trulysafe/-pedantic argument to the V compiler simply turning all panic( ... ) calls into return error( ... ) and thus forcing one to write ! (or or{}) everywhere explicitly.

I.e. no new syntax, no new semantics. Only one additional compiler argument making a very simple AST transformation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing return type by passing extra cli argument is quite hard accomplish.
return type is always hardcoded in function definition. return error(...) requires ! in return type.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ken0x0a I did not try to do any effort estimation but I am 100% certain it is doable and that it would not slow down the compiler much (if at all). V's AST shall allow for such transformations already in its current form.


V may require an new "stricter" compilation option forcing developers to handle all possible optional option/result.

It may be that `??` could be used when the absence of handling is fine and `!?` used when the handling of the `or` case must be performed when compiling using the stricter mode.
Copy link

@dumblob dumblob Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds a bit like what I proposed above 😉. Though I dare to say your proposal is needlessly complicated with actually not gaining anything more? But I definitely might be wrong as I do not follow V development that closely.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, maybe ??? could mean did you run my code or not, and ???? could mean no, really did it run and ????? could mean come on, tell me already - did you run it or not, and ?????? could mean...

b := to_integer('abc') // 0
c := to_integer('abc') or { 100 } // 100
d := to_integer('abc') or { print(err); return } // displays 'this is not a number'

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rather not. This looks so fishy and implicit that it hurts 😢.

@thomas-mangin
Copy link
Contributor Author

thomas-mangin commented Jun 30, 2022 via email

@dumblob
Copy link

dumblob commented Jul 1, 2022

Ok, this motivation to handle currently unhandled panics originating from non-function expressions is fine and I totally want to change this situation.

From my point of view such expressions or language built-in features are just a fancy ways of function calls or intrinsic "invocations". Therefore I stand by my proposal of using ! (or ?) for them. I see really no technical difference and thus consider these not orthogonal concepts.

Actually I am a long time very strong proponent of getting rid of panic() altogether (in favour of the solution currently used for maps etc. ideally accompanied by the -trulysafe argument I proposed above).

@spytheman
Copy link
Member

The concept introduced is a way to allow the caller to optionally not
handle errors when returned by a called function.

I really do not get it - so with the new proposal, some calls could be more relaxed (not checked by the compiler), not more strict. How would that help with the scenario, where a request failed - you still need to react somehow?

Panic-ing just makes the process exit, which then can be handled by a process runner like runit or systemd. Administrators can configure the process runner to restart the process, potentially after a small delay to prevent other problems, and/or send a notification, and/or log the failure etc.

@thomas-mangin
Copy link
Contributor Author

@spytheman Thank you for trying to understand my line of thought.

To not have to panic, when []int{len: 10000, cap: 30000} fails, it needs to be returned to the user to handle. I would think that most users would expect something like v := []int{len: 10000, cap: 30000} or { ... } for the syntax. This part is probably uncontroversial.

Thinking about the implementation, functions such as __new_array in vlib/builtin/array.v do use malloc to generate the underlying C. So this panic must be removed and an error returned.

As all this is internal to V: all good. And a solution could probably be found without using the syntax presented in the RFC.

I then considered that it would probably be unpopular to ask developers to always add the or {} block when memory panic is so rare. V is nicer than Go in that aspect as it does not require as much error handling boilerplate (which could be otherwise added using v fmt).

What is then considered here is optional error handling, and I thought it could be generalised, and this is what the RFC presents with ?? (It may even help within vlib for the implementation of the case above, but we can ignore that for now as it is speculation).

If the error handling is then optional, then a programmer could forget to write an or {} block by accident. This is why I thought that the enforcement of the check should probably be behind a 'strict' compiler option and !? could be used to indicate that it should not be ignored in strict mode. ?? and !? would be the same in the normal, 'forgiving' mode.

If we put aside the two compilation modes, we are left with only ?? as a way to indicate that the compiler should allow the caller to not require an or {} block in case of failure, which seems to be a useful feature.

In particular, if it is later extended to something like ??int which does indicate that you would always get an int but that some of the value could be the result of an error and therefore allow default behaviour and again reduce boilerplate error handling code but letting the developer decide what to do.

Obviously, it could be expressed syntactically differently, I have no shares in ??, it just looked like a logical option to present the concept.

@thomas-mangin
Copy link
Contributor Author

so with the new proposal, some calls could be more relaxed (not checked by the compiler), not more strict. How would that help with the scenario, where a request failed - you still need to react somehow?

I proposed that we could indeed let the developers indicate how the caller should handle this issue and request that it is not ignored, but make it optional behind a compilation flag to keep backward compatibility. My explanation above may have helped with this question.

Panic-ing just makes the process exit, which then can be handled by a process runner like runit or systemd.

Correct. This is what would have to happen today.

Administrators can configure the process runner to restart the process, potentially after a small delay to prevent other problems, and/or send a notification, and/or log the failure etc.

Also correct and also not always acceptable. It is pragmatic and fine in many cases but not always.

If a global panic can happen during what should be atomic operations then something bad is due to happen at some point.

I used a web server as an example before but perhaps I should have used a database: leaving the data in a broken state following a memory allocation failure during a new inbound connection should not happen.

The aftermath could require a recovery tool and a painful audit of the data on disk: not something systemd can handle.

@dumblob
Copy link

dumblob commented Jul 3, 2022

@thomas-mangin thank you for taking the time to explain in detail your train of thought! I now see where the disagreement originates from.

If the error handling is then optional, then a programmer could forget to write an or {} block by accident. This is why I thought that the enforcement of the check should probably be behind a 'strict' compiler option and !? could be used to indicate that it should not be ignored in strict mode. ?? and !? would be the same in the normal, 'forgiving' mode.

This is it 😉. I strongly disagree with the fact that the language shall provide a way to enforce anything at the callee site (i.e. definition/declaration in V) instead of at the caller site (i.e. invocation). Historically this always in every single language offering such enforcement proved wrong and every such language sooner or later added a way to circumvent it (or alternatively such language did not become popular enough to get to this point) effectively zeroing the added value of the enforcement.

And the reason?

Simply because you - as an API designer - never and I repeat never know the future. Worse than that, you do not even know whether there might be an exception to an exception in the very same app you are writing right now with this shiny new "enforcing" API. Even if it would be just for debug purposes to save time or for automated testing or simply to solve a new problem your manager will come with tomorrow, you must not strictly enforce anything at the callee site.

Memory allocation is a perfect example - you might want to make one (re)allocation API call failing in some critical section but over time you will realize you want in the whole rest of the app make the very same (re)allocation API call just best-effort. How will you do that if the callee would "enforce" it? Would you want to always type or{} everywhere?

I would say such cat & mouse game is not what we are after.

I bet you want to have a safe app. In that case if V adopted the map[x] behavior with an optional or{} clause for everything which can panic() nowadays (i.e. not safe by default 😢 but read further), then the only thing missing would be v vet enforcing certain places (based on a "selector" of source code piece(s) to check - could be a module name(s) or a given file(s) or a given regex pattern(s) matching only certain functions or whatever) to have the or{} clause.

Your use case would then be covered with v vet -nopanic which without any selector would simply check everything and bail out on every single place where or{} is only optional but present in the code.

Would this be safe enough for you @thomas-mangin ?

@thomas-mangin
Copy link
Contributor Author

thomas-mangin commented Jul 4, 2022

Any solution which will ensure that no panic can occur due to memory allocation and let me handle the problem is good.

Any solution which allows me to not return not simply errors, but also a default value would also be welcomed.
I remember Alex pointing out that he was not pleased with the number of or {} and the like in V code, which is what initially made me consider the idea behind the RFC.

My previous post was to help to read the RFC, not a justification of it. The discussion slipped into opinions, but I appreciate @dumblob attempt to explain the reasoning behind his reservations/objections, even if I could also not follow them all.

I am still happy to answer questions and update the RFC as required if the request made is clear enough for me to act on (please take note that I can not edit the RFC here and will have to create a PR like everyone).

@thomas-mangin
Copy link
Contributor Author

thomas-mangin commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants