Add support for failable mark to function declarations

[mks-h]

Makes it a requirement to declare failable functions as such, using ? failable mark.

Syntax:

fun test(): Null ? {
    fail 1
}

Resolves #272

[mks-h]

Right now there are two versions:

1. On commit 13f278c it is required to mark failable functions, and forbidden to mark non-failable functions.
2. On commit eea62eb non-failable functions will be considered failable, if marked.

Which route should be taken here? If we allow declaring non-failable functions to be failable, then should a warning be emmited? If so, should there be a compiler flag to remove it?

[mks-h]

Allowing to mark non-failable functions can be seen as a feature to allow API stability. However, this obviously allows for misuse — having too many pseudo-failable functions in libraries would make it more annoying to use them.

Maybe for now it is a good idea to stick to the more strict but minimal rule of "only failable functions must be marked". Then, it would be possible to relax it, if ever needed.

[Mte90]

The difference with unsafe is...?

In this way when we will improve docs we can explain it.

[b1ek]

The difference with unsafe is...

...that they are completely different things

unsafe marks a command execution statement as failable, while this marks a function failable in the function declaration.

it will help for user to know if the function is failable or not, and probably open up a possibility for handling a runtime error in the function (like $...$ failed {})

[mks-h]

To clarify, this change does not add any new features. It only creates a requirement to mark failable functions with the ? in their signatures. This is purely for readability purposes, so that it is easy to differentiate these functions without looking into their bodies, or waiting for compiler to yell at you for missing the failable block.

Well, the last commit does add a new feature of marking any function with the ? to make them failable, but I'd rather not merge it, at least not yet.

[b1ek]

@mks-h so it kinda just ignores the ? symbol in there so it wont throw a syntax error?

[mks-h]

In the first case, it throws an error if there's a ? mark on a non-failable function, or no ? on a failable one. In the second case, if there's a ? on a non-failable function, instead of erroring out, it turns the function into a failable one — so you have to call it like one.

P.S. Technically it changes the "failable" property to true, instead of just ignoring the mark.

[b1ek]

ah, so it would just pass an error along if there is a $...$? block inside the function

[mks-h]

ah, so it would just pass an error along if there is a $...$? block inside the function

I'm not sure what you are asking. Can you elaborate?

As a general answer — the way you use failable functions doesn't change. Whatever is considered a failable function now, also doesn't change. In the first variant, the one I prefer, the only thing that changes — is that if a function is already failable (per whatever definition of a failable function there is), you are also required to put the ? mark after the return type in the function signature — fun some_func(): Text ? { (the one after text). This mark does not change or add anything. It is required only to increase readability — so a user of the function can know whether it is failable by looking for the ? after the return type. And it is also forbidden to put ? in there, if the function is not failable. These two rules are enforced by the compiler, so if you break them — the compiler will not finish compilation, and will return an appropriate error.

In the second variant, the last rule about forbidding to use the ? mark on a non-failable function is removed. Instead of erroring out, the compiler will now consider the marked function to be failable, even if it really isn't. So you will have to use this function as any other failable function, except there will never be an error, and therefore whatever error-handling logic you have for it will never actually run. That is unless the function's implementation changes in a future update to actually return an error in some case — and this was the whole idea, to allow future-proofing library's API. But again, after implementing it, I've decided that I'm against this idea, at least until we receive some actual request for it.

[Ph0enixKM]

@mks-h Let’s implement the strict version now which disallows the scenario to mark an infailable function a failable.

fun foo(): Null? {
    // This will compile if and only if the function can actually fail
}

[Ph0enixKM]

Some required changes

[Mte90]

You need to align with the latest changes.

[mks-h]

Re-implemented the PR to have a Type::Failable(Box<Self>) written as Text?. This type, "a Failable type", is required to be used as a return type in a failable function. Using it as a return type on a non-failable function is disallowed, and so is using it as an argument type.

[mks-h]

You need to align with the latest changes.

The latest changes remove the ability to write should_panic tests that succeed if the test panics with a specified error message — they are a very important part of this PR. In order for me to rebase, you'll need to do something with that.

[Ph0enixKM]

@mks-h You can add the error tests to a separate rust file tests/errors.rs. We can create a new issue to implement the file-based tests for errors too.

If you move the tests - I will approve and we will merge this request

[mks-h]

Thanks to new tests, I've discovered bugs related to making Failable a data type. I've managed to fix them in one sweep by writing custom PartialEq implementation for Type, where Failable is equal to anything else that is or has the same inner type. But honestly, this sounds like a hack, so I'll be looking into it further.

[Mte90]

In the meantime I approved for the CI so we can see if it pass or not :-)

Edit: Pass

[Mte90]

So with a cargo test I don't see this new tests, fixed that with a commit.

[mks-h]

So with a cargo test I don't see this new tests, fixed that with a commit.

Oops, I forgot to actually enable the error tests. But by the "new tests" I meant that some tests have been added to the master branch, so when I rebased — a few (not mine) tests have failed.

IIRC, one issue was that the actual return type was a non-failable variant, while the declared return type was a corresponding Failable type — so they were different, and therefore the test didn't succeed. Another issue was similar in nature, but with the main block.

As a quick fix I've implemented manual PartialEq for Type to consider a Failable type the same as the corresponding non-Failable one. But I'll look into it further, maybe there's a better fix.

done

[Ph0enixKM]

Just one relatively small change

[mks-h]

The issues were happening at the return statement's type check. Instead of the former workaround, I fixed them by making it compare the actual type with the inner type of the declared Failable type.

[Ph0enixKM]

And we get back that PartialEq. Great job @mks-h! Well done. I'll do the last CR later today. But when I quickly checked your changes it all looked good. Thanks for your work!

[mks-h]

Updated to properly handle casts. The next thing to do, is to make handlers of failable things (unsafe, failed, and expr?) change the type of an expression from T? to T.

done

[Mte90]

I changed this pr to draft as you said that is missing some stuff so we are sure that do't get merged if not ready :-)

Thanks for your work!

[mks-h]

I'm fairly confident this is it:

• Made "failed" expression (+ block & "unsafe" modifier) unwrap the Failable type
• Made commands return "Text?" instead of "Text" — though, technically this does nothing but a Box allocation
• Refactored tests a little

[Ph0enixKM]

Some changes required

[Ph0enixKM]

Great job @mks-h 👏👏👏 solid work. Thank you for your contribution