throw and catch with SkipMacroException

[straight-shoota]

SkipMacroException is used in the to signal a skip_file macro expression. This is not actually an error. The exception is used to work like throw/catch in Ruby.

We don't have throw/catch in Crystal and I think it's for good reasons (https://forum.crystal-lang.org/t/throw-and-catch/256). Yet we make use of this concept in the compiler. That's definitely a hack because there's no error involved, but it necessarily inherits from Exception which is the base error type.

A side effect of that is that the compiler requires exceptions to function, not just for error handling. This poses difficulties when porting the compiler to new targets where exception support is still missing (#10870 (comment)).

I think the right solution would be to remove SkipMacroException and properly implement skip_file with an other form of signalling that is embedded in regular code flow. This is probably not going to be trivial because it needs to be handled through several levels of compiler layers.

Alternatively, we could decide this might actually be a good use case for this pattern and we want to keep it. Then we should make sure that it's properly implemented (non-error exception base type, skip the stack trace). Maybe this could also warrant adding throw/catch feature to the language.

[asterite]

Yet we make use of this concept in the compiler

I don't quite agree. We use exceptions for control flow. That's contrived, but valid. People do this in languages with exceptions, it's unrelated to Ruby's throw/catch.

An alternative to using exceptions is requiring that skip file happens as the first node in the file. I think that kind of makes sense because skip_file means "skip the entire file" so having it in the middle of the file is awkward. So we could check if the first node is a macro node, and check if it contains a call to skip_file. If so, we evaluate that. Otherwise, we use regular file evaluation and if we find a skip_file we give a compile error saying that skip_file must happen in the beginning of the file.

[asterite]

Yet another alternative is to deprecate skip_file and stop using it. You can achieve the same thing by not requiring a file.

[HertzDevil]

Here are the files that use skip_file after the first {%:

• spec/compiler/semantic/macro_spec.cr: The specs that test skip_file place them in the middle of the test code.
• spec/std/thread_spec.cr, spec/std/thread/condition_variable_spec.cr, spec/std/thread/mutex_spec.cr: They add a pending test on musl.
• src/io/encoding_stubs.cr: Has a flag check, can be rewritten using a suffix conditional.
• src/io/encoding.cr: This in turn requires encoding_stubs.cr and then calls skip_file immediately, suggesting that the whole part can be easily refactored into separate files.

The most compelling use case seems to be excluding files from a glob via require or implicitly via crystal spec.

[asterite]

I also wonder whether anyone outside this repo is using skip_file. My guess is no, because we use it for platform-de pendent code. In that case, we could refactor our code to not use skip_file and then silently remove that macro method.

[HertzDevil]

A quick search shows that it is indeed used:

{% unless flag?(:anyolite_implementation_ruby_3) %}
  {% skip_file %}
{% end %}
# ...

{% skip_file if Avram::Model.all_subclasses
  .map(&.stringify)
  .includes?("UserOptions")
%}

{% skip_file unless Lucille::JSON.includers
  .map(&.stringify)
  .includes?("UserSettings")
%}
# ...

# ...
{% skip_file if @top_level.has_constant? "Spec" %}
# ...

[straight-shoota]

I'm not sure it would be such a great idea to require skip_file be the first expression. It's a strong restriction and could be really awkward. What about comments, for example?

Also I think it shouldn't be too difficult to handle later in the file.
In order to remove the control flow by exception, we need a way to stop evaluating the current file. I don't think it makes much of a difference whether that only takes the first expression into account because the entire expression including skip_file needs to be processed, that means there's usually already a number of sub-expressions (conditionals, logic operations, flag? calls) involved. I'm pretty sure the implementation would not be very more complicated when it's not limited to the first expression.

[asterite]

Sounds good.

Also note that for example autocasting of numbers/literals is done using exceptions.

I think it's just simpler to spend that effort implementing exceptions on the targets we want to port the language to.

[lbguilherme]

I'm going back and forth on this and my current opinion is that exceptions is a feature of the language like any other and the compiler shouldn't be restricted from using it. Targets that don't support exceptions won't have a native compiler for them and will have to live with cross compilation until they support the full language. It doesn't stop programs being compiled for said target.

If this is changed then it should be based on code clarity or performance on the grounds that exceptions should not be used for normal control flow. That said, I don't have a problem with exceptions being used for control flow.

[straight-shoota]

The compiler uses another exception type, RetryLookupWithLiterals for control flow as well. That's happening much more frequently than SkipMacroException. We improved performance in #15002 by assigning a static, empty call stack to these exceptions (control flow management is not exceptional and we don't care about the call stack).

We could probably do the same for SkipMacroException, although it won't have much effect on performance since skip_file is used very rarely.

This again raises the question whether we should generalize the concept of control flow via stack unwinding.
For example, we could introduce an exception-like base type which doesn't have a call stack (or initializes it to empty like #15002) and use that as common parent for RetryLookupWithLiterals and SkipMacroException.