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

Better document backtrace re-raise -- was: Backtrace changes when referring to a nullary exception constructor... #7375

Open
vicuna opened this issue Sep 27, 2016 · 16 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented Sep 27, 2016

Original bug ID: 7375
Reporter: @johnwhitington
Status: acknowledged (set by @gasche on 2016-09-28T12:18:47Z)
Resolution: open
Priority: normal
Severity: text
Version: 4.03.0
Target version: 4.07.0+dev/beta2/rc1/rc2
Category: documentation

Bug description

The following code in a try...with block:

 | Exit as e ->
     if !debug then raise e;
     exit 0

gives:

Raised at file "eval.ml", line 615, characters 27-31
Called from file "ocamli.ml", line 232, characters 10-25
Called from file "ocamli.ml", line 292, characters 21-26
Re-raised at file "ocamli.ml", line 300, characters 30-31
Called from file "ocamli.ml", line 307, characters 31-36

But the original code, which ought to do the same:

 | Exit ->
     if !debug then raise Exit;
     exit 0

gives just:

Raised at file "ocamli.ml", line 300, characters 30-34
Called from file "ocamli.ml", line 307, characters 31-36

@vicuna
Copy link
Author

vicuna commented Sep 27, 2016

Comment author: @gasche

The logic to distinguish "raise" for "reraise" has changed in recent OCaml versions. There is no clear specification, so it is not easy to say what is simply unfortunate and what is an actual bug.

If you want to reliably get a "reraise" expression, you can express this intent by using the "reraise" primitive:

external reraise : exn -> 'a = "%reraise"

...

 | Exit as e ->
     if !debug then reraise Exit;
     exit 0

My gut feeling is "not a bug" on this one, although we could wait for others' opinion -- and it would be interesting to know if the proposed workaround is sufficient.

@vicuna
Copy link
Author

vicuna commented Sep 27, 2016

Comment author: @johnwhitington

Thanks.

Yes, this works fine. Can we have it in the Standard Library, please? I can't see where it would go, but perhaps Printexc is the least-illogical place.

Looking at the original question in a different way:

 | Exit as e ->
     if !debug then raise e;
     exit 0

 | Exit ->
     if !debug then raise Exit;
     exit 0

Why should these two pieces of source generate different code? As of recent versions of OCaml, nullary exception constructors are not freshly allocated, I thought?

@vicuna
Copy link
Author

vicuna commented Sep 27, 2016

Comment author: @gasche

The big change in handling of (re)raise was work by Alain Frisch in October 2013, that went in 4.02.0. The implementation of reraise was changed from a dynamic check (of equality of exceptions) to a compile-time source-to-source transformation. The compiler now recognizes the specific pattern raise <the-identifier-of-the-exception>, instead of doing an equality check, which would indeed succeed in recent OCaml versions as here you have e == Exit.

@vicuna
Copy link
Author

vicuna commented Sep 27, 2016

Comment author: @alainfrisch

The "trouble" is indeed that with the improved representation of constant exceptions (which used to be freshly allocated every time the constructor was referenced), only checking dynamically for physical equality no longer works: there would be way too many false positive, i.e. cases where the runtime system would consider a "fresh" raise to be a reraise and happily push more frames to an unrelated trace. So the criterion is now a combination of physical equality and the fact that the compiler detects syntactically a reraise (i.e. a raise on a variable bound by an exception pattern around the current scope). This is still imperfect, since the execution withing the exception handler could internally trigger a physically equal exception (simply calling a function which internally raise e.g. Not_found); this would indeed create weird traces. But the syntactic criterion avoids many such false positives.

Several improvements have been proposed on that scheme recently (allowing an explicit reraise which would not check for physical equality; and allowing to capture explicitly the trace in an exception handler and reuse it for a later reraise).

@vicuna
Copy link
Author

vicuna commented Sep 27, 2016

Comment author: @alainfrisch

Can you elaborate on which case the current behavior doesn't work for you? You could indeed bind the constant exception with an [... as exn] pattern and raise this variable. This should work except if you need to reraise outside the syntactic scope of the exception handler.

@vicuna
Copy link
Author

vicuna commented Sep 28, 2016

Comment author: @johnwhitington

Oh, the current behaviour works fine for me, now that I know about it. I was just surprised!

@vicuna
Copy link
Author

vicuna commented Sep 28, 2016

Comment author: @gasche

That might suggest that we need more documentations for these kind of things. That's clearly an aspect of the language behavior that affects people, has changed a lot recently (and may keep changing), and that doesn't seem documented anywhere -- whenever there are questions about it we need "people in the know" to comment.

@vicuna
Copy link
Author

vicuna commented Sep 28, 2016

Comment author: @gasche

I'll mark this not-quite-a-bug "resolved" when I have a clearer idea of how to go forward documentation-wise.

@vicuna
Copy link
Author

vicuna commented Sep 29, 2016

Comment author: @damiendoligez

We could keep this PR open, change its category to "OCaml documentation", and change the title.

@vicuna vicuna added this to the 4.07.0 milestone Mar 14, 2019
@vicuna vicuna added the bug label Mar 20, 2019
@github-actions
Copy link

github-actions bot commented May 9, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 9, 2020
@Lupus
Copy link

Lupus commented May 27, 2020

I've just landed to this PR while googling for "OCaml reraise". Would be awesome to find the documentation page with clear explanation of the semantics for raise and re-raise instead :)

Hope this comment keeps the bot away from this issue for a bit.

@github-actions github-actions bot removed the Stale label Jun 10, 2020
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jun 16, 2021
@Octachron Octachron self-assigned this Jun 16, 2021
@Octachron Octachron removed the Stale label Jun 16, 2021
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jul 1, 2022
@johnwhitington
Copy link
Contributor

Still an issue.

@github-actions github-actions bot removed the Stale label Jul 4, 2022
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jul 7, 2023
@johnwhitington
Copy link
Contributor

Still an issue. (For clarity, I don't have the expertise to document this).

@github-actions github-actions bot removed the Stale label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants