make __crystal_raise_overflow error more clear

[ghost]

per #8437, make the "__crystal_raise_overflow" error be more explicit about what's actually happening

with this pr, running the shell commands in that bug report (echo 2+2 > test.cr && crystal build test.cr --prelude=empty) produces the following output:

LLVM function `__crystal_raise_overflow` is not defined (are you using a prelude?) (Exception)
  from src/compiler/crystal/codegen/codegen.cr:2039:9 in 'crystal_raise_overflow_fun'
  [long stack trace]
Error: you've found a bug in the Crystal compiler. Please open an issue, including source code that will allow us to reproduce the bug: https://github.com/crystal-lang/crystal/issues

that bug report had some confusion as to whether this was really a bug, and as to what the original error message (BUG: __crystal_raise_overflow is not defined) meant. with this pr, the final line still claims there was a compiler bug that should be reported, but at least the error message is imo a bit clearer

i don't think this should report a compiler error, but i have no idea where to go about changing that, and trying to figure it out lead me down a rabbit hole

[bcardiff]

I think that something that tells exactly what it can be done would be better.

WDYT?

[ghost]

i think that's much better than what i originally wrote. mind if i commit your change exactly?

[bcardiff]

Sure @3n-k1, you can apply the suggestion directly from github or adapt as you prefer.

[asterite]

Could someone explain to me how to define that method? I mean, if we are telling the user to require prelude or define the method, how will they know how to do it?

I think the main issue is having the ability to specify another prelude. We dev compilers use it to reduce bugs so they don't depend on the std. Because it's a compiler dev tool it should probably be a super rare option, not something that you want users to do.

There's no way to change the prelude of Ruby, Java, C#, etc. You can change the prelude of Haskell, but nothing in the prelude connects to base language constructs like in Crystal.

So my proposal is either remove the option to use a different prelude or make any prelude work without having to define these secret functions.

[ghost]

@asterite i think making the compiler not depend on a prelude would be best

personally, i was just removing the prelude to reduce the lack of emitted llvm so i could inspect it easier; adding the prelude in generates an enormous amount of llvm-ir. when searching the error online, i ran into some others who were confused by it like i was

[bcardiff]

I think the value of this PR is that when someone is playing with low level stuff it and accidentally reaches an overflowing operation, it will fail in a more useful way.

When porting the compiler to a new platform exceptions is not the first thing to port, but math operators are. The user is left to use &+ ops or at least define fun __crystal_raise_overflow : NoReturn as LibC.exit(1).

Another use case where we can reach this, is when wanting to analyse the llvm-ir. There the prelude adds way too much noise and disabling it helps, but again you need to be careful of avoid overflowing operators (or define __crystal_raise_overflow).

There are many assumptions of the std-lib by the compiler, I would like to move them to a core part of the std-lib. But still, I think there is value in failing in more controlled way in case someone.

[ghost]

@bcardiff can confirm, LocationLess exception looks much better than the previous stack trace

[ghost]

if that looks good, lmk and i'll rebase

[bcardiff]

Thanks @3n-k1, there is no need to rebase. It will be squashed eitherway.

[ghost]

hmm, should we add a little bit in the error message to let the user know that instead of using a prelude, they can just use something like &+ instead of +?

[jhass]

I wouldn't go beyond adding something like "or avoid calling methods that may raise". Not strictly necessary though IMO

[bcardiff]

I don't think is needed. That would be changing the semantics of the code, and without pointing the location of the user code that triggered this call the suggestion is hardly actionable.

I'm satisfied with the current PR. Thanks

[ghost]

awesome, thanks for working with me on this :)