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

Use of @@ seems to trip backtrace line reports #6920

Closed
vicuna opened this issue Jun 27, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Jun 27, 2015

Original bug ID: 6920
Reporter: @dbuenzli
Assigned to: @diml
Status: closed (set by @xavierleroy on 2017-02-16T14:16:24Z)
Resolution: fixed
Priority: normal
Severity: minor
OS: macosx
OS Version: 10.10.3
Version: 4.02.2
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: ~DO NOT USE (was: OCaml general)
Tags: patch
Has duplicate: #6258
Monitored by: @gasche @diml @ygrek @hcarty @dbuenzli

Bug description

Behaviour was tested with both 4.02.1 and 4.02.2.

The use of @@ sometimes reports the backtrace on the next line, see the steps to reproduce.

Steps to reproduce

ocamlc -version
4.02.2

The following backtrace wrongly report line 4

cat why_at.ml
let why : unit -> unit = fun () -> failwith "WHY?"
let f () =
why @@ ();
ignore (3 + 2);
()

let () = f ()

ocamlc -g why_at.ml
./a.out
Fatal error: exception Failure("WHY?")
Raised at file "pervasives.ml", line 30, characters 22-33
Called from file "why_at.ml", line 4, characters 2-22
Called from file "why_at.ml", line 7, characters 9-13

If I remove the @@ the report is correct

cat why.ml
let why : unit -> unit = fun () -> failwith "WHY?"
let f () =
why ();
ignore (3 + 2);
()

let () = f ()

ocamlc -g why.ml
./a.out
Fatal error: exception Failure("WHY?")
Raised at file "pervasives.ml", line 30, characters 22-33
Called from file "why.ml", line 3, characters 2-8
Called from file "why.ml", line 7, characters 9-13

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 27, 2015

Comment author: @dbuenzli

Also sometimes use of @@ suppresses the location. Here again removing the @@ allows to regain the location.

cat why_swallow.ml
let why : unit -> unit = fun () -> failwith "WHY?"
let f () =
for i = 1 to 10 do
why @@ ();
done;
ignore (3 + 2);
()

let () = f ()

ocamlc -g why_swallow.ml
./a.out
Fatal error: exception Failure("WHY?")
Raised at file "pervasives.ml", line 30, characters 22-33
Called from unknown location
Called from file "why_swallow.ml", line 9, characters 9-13

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 28, 2015

Comment author: @gasche

Currently the implementation of (@@) and (|>) is using primitives that are optimised (early in the compiler passes). I think I would be in favor of just implementing them with their obvious definition, to get rid of these issues.

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 29, 2015

Comment author: @damiendoligez

@gasche but it's important to guarantee that they incur no run-time overhead.

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 30, 2015

Comment author: @diml

Looking at the -dlambda diff, in both Daniel's examples the problem comes from a missing "after" event when using @@:

--- /tmp/good.lambda 2015-06-30 11:21:22.349788092 +0100
+++ /tmp/bad.lambda 2015-06-30 11:21:27.636788092 +0100
@@ -10,7 +10,7 @@
(function param/1013
(funct-body truc.ml(2):57-97
(before truc.ml(3):64-97

  •         (seq (after truc.ml(3):64-73 (apply why/1008 0a))
    
  •         (seq (apply why/1008 0a)
              (before truc.ml(4):77-97
                (seq (ignore (+ 3 2)) (before truc.ml(5):95-97 0a)))))))
    match/1012 = (after truc.ml(7):108-112 (apply f/1009 0a)))
    

Looking at translcore.ml this happens only when the primitive is applied to as many arguments as its arity. For instance when adding one argument to [why] we regain the correct location:

let why : unit -> unit -> unit = fun () () -> failwith "WHY?"
let f () =
  ( @@ ) why () ();
  ignore (3 + 2);
  ()

let () = f ()

Adding [Pdirapply] and [Prevapply] to [Translcore.primitive_is_ccall] fixes this issue. Looking at the code I'd say it is safe to do so but I'm not 100% sure. I attached a patch.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 28, 2015

Comment author: @xavierleroy

The proposed patch looks reasonable to me. Jeremy, if you're still happy with this code, please commit it to trunk. (And mark this PR as resolved.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 30, 2015

Comment author: @diml

OK, committed in trunk with Daniel's tests (62fb2c5).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.