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

Never() factory constructor #3280

Open
rakudrama opened this issue Aug 16, 2023 · 7 comments
Open

Never() factory constructor #3280

rakudrama opened this issue Aug 16, 2023 · 7 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@rakudrama
Copy link
Member

I often find myself wanting to mark some point in the code a unreachable. The type system often requires something, but the reason that the path is infeasible that is outside of what can be expressed in the type system. The usual remedy is to throw an exception, with there being a choice of many corelib errors - UnimplementedError, UnsupportedError, StateError and AssertionError.

Using UnimplementedError is nice since it does not require inventing a message.

    throw UnimplementedError();

However, the documentation stays UnimplementedError is not really for this purpose: "If the class does not intend to implement the feature, it should throw an UnsupportedError instead.". And it is confusing since it leads to the question 'what is not implemented?'.

So I might use UnsupportedError. That requires me to invent a message.

    throw UnsupportedError('Unreachable');

This too is not an ideal semantic match, since this kind of error is for when "The operation was not allowed by the object.". There is often no operation that can easily be identified, or object that is to blame; rather there is some intended invariant that must have been broken.

A StateError is not appropriate...

    throw StateError('Unreachable');

...because it "Should be used when this particular object is currently in a state which doesn't support the requested operation, but other similar objects might, or the object itself can later change its state to one which supports the operation."

Perhaps the closest to my intent is to throw an AssertionError, again I have to come up with a message:

    throw AssertionError('Unreachable');

It feels a bit odd to for the message to be 'Unreachable' when assertions usually describe what is expected rather than what went wrong, but AssertionError('reachable') feels even worse - the problem is not that something is unexpected when I am here, I'm not supposed to be here in the first place.

A final throw-based pattern is to not throw an Error at all:

    throw 'Unreachable';

Some projects have coding standards that forbid this.

Since there are five slightly unsatisfactory and somewhat verbose ways to indicate that control flow should not reach somewhere, we should add another one that is at least concise and directly tied to the type system:

    Never();

The Never() factory constructor throws an error to ensure that the expression does not have a value.

Never() succinctly expresses that I don't expect to get here and does not require that I invent a message that gets baked into the compiled program. When I have to write throw SomeError('Unreachable') I am writing 'unreachable' in three different spellings when once should be sufficient.

I don't really mind exactly what kind of error is thrown, and if the expression Never() is explained as a factory constructor, we should probably make it work like other factory constructors and permit a Never.new tear-off.

@rakudrama rakudrama added the feature Proposed language feature that solves one or more problems label Aug 16, 2023
@ykmnkmi
Copy link

ykmnkmi commented Aug 16, 2023

I throw AssertionError('Unreachable') too.

Or add unreachable statement like break.

@Jetz72
Copy link

Jetz72 commented Aug 16, 2023

In the few cases I've had to do this, I've found AssertionError is suitable for the job. I generally make the error a quick summary of why that part of the code should be unreachable, or which assumption had to have failed in order to get to that point - e.g. throw AssertionError("nextState should never be 'ready' while current state is 'working");`

If you want to avoid having to write out messages in each case though, you could also just subclass AssertionError or Error to add in a default parameter, or have a common function for throwing one:

class UnreachableError extends AssertionError
{
  UnreachableError([super.message = "This should never execute."]) : super();
}

Never get unreachable => throw AssertionError("This should never execute.");

Either of those seems like they'd communicate the intention more clearly than invoking Never(). (Though I'm generally a fan of finding excuses to add common class behaviors to the built in special classes, so I wouldn't actually be opposed to a doomed-to-fail Never constructor.)

@lrhn
Copy link
Member

lrhn commented Aug 16, 2023

I'd go with AssertionError too, or just throw "unreachable";. It doesn't matter what it says it will throw, if it's never executed, right?

If we introduce factory Never() => throw ....; what should it throw?
And if it's good enough, why don't you just throw that yourself?

Or make yourself a helper function:

Never get unreachable { 
  assert(false, "Unreachable"); // With message in debug mode.
  throw AssertionError(""); // Without message in production.
}

(I don't know if this is technically a langauge issue, it's just asking for a single method/constructor to be added to an existing type type, a method which doesn't have to be mentioned in the language specification.)

@rakudrama
Copy link
Member Author

@lrhn I thought it should be a language issue since Never is special. It not a regular interface type or a typedef of something else. There is no definition in dart:core where one might add the factory constructor.

There was a recent issue about identifiers that denote types being treated differently depending on the type.
One can write Never.toString() (and T.toString()), but not String.toString(), the latter being a reference to a non-existent static member or constructor.

It appeared that there was a desire that the type literals should be treated more uniformly.
That would require (Never).toString() and (T).toString() to align with (String).toString().

And if it's good enough, why don't you just throw that yourself?

I can (and do) do that, but I see Never() as an opportunity for the language to be more regular and more useful.

Or make yourself a helper function:

This kind of helper function has no obvious right library to be declared in, except dart:core, otherwise in order to help me write more-concise yet clear programs, I need to write more code, and then import it! Even if you were excited to add that definition of unreachable to dart:core, it is something novel, and unrelated to other things, that a working programmer has to learn, rather than Never() which can be thought of a aspect of what Never means - unreachability is a concept in the language.

@modulovalue
Copy link

I would love to see a new Error in dart:core (such as UnreachableError) that is meant to be thrown in cases where the type system is not expressive enough to encode that a branch is not meant to ever be reached (and Gödel says that such an Error will be needed until the end of time!)

I think that constructing a Never() is not clear about the intent and could be confusing. However, something like:

else {
  throw UnreachableError(optionalMessage);
}

would be unambiguous, clear, and could contain a descriptive message as a doc comment on the declaration of UnreachableError.

I'd go with AssertionError too, or just throw "unreachable";. It doesn't matter what it says it will throw, if it's never executed, right?

AssertionErrors could be confusing to people because asserts are not enabled in release builds of, e.g., flutter apps. throw "unreachable" would lead to diagnostics because a popular lint discourages throwing strings: only_throw_errors.

I also think that implicit Never-related control flow should be discouraged.

A huge +1 for UnreachableError from me!

@mateusfccp
Copy link
Contributor

@modulovalue Why would it have an optionalMessage if the branch is unreachable?

@modulovalue
Copy link

It is not unreachable, that is, we haven't provided a proof that the type system is able to verify. We only assume it to be unreachable.

It is easy to introduce a change that invalidates the assumptions that made one believe that that branch is unreachable.

Therefore, I have found it to be valuable to explain why I believe such a branch to be unreachable.

throw UnreachableError(
  """Under the assumption A, B and C, this is unreachable. Consider 
  checking whether the assumptions hold if this branch is ever reached.""",
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

6 participants