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

Delayed effects in partially applied functions #7531

Closed
vicuna opened this issue May 8, 2017 · 6 comments
Closed

Delayed effects in partially applied functions #7531

vicuna opened this issue May 8, 2017 · 6 comments

Comments

@vicuna
Copy link

vicuna commented May 8, 2017

Original bug ID: 7531
Reporter: jmi
Assigned to: @mshinwell
Status: resolved (set by @mshinwell on 2017-05-08T11:17:10Z)
Resolution: duplicate
Priority: normal
Severity: minor
Version: 4.04.0
Category: middle end (typedtree to clambda)
Related to: #7533

Bug description

The native code compiler erroneously delays the effect of an operator until (if ever) it is fully applied.

The following program outputs a newline when compiled with the bytecode compiler but not with the native code compiler:

(let _i = print_newline ()                                                                                    
 in fun q -> fun i -> "") ()

I believe the issue is described by the following comment on line 828 of asmcomp/closure.ml:

(* We convert [f a] to [let a' = a in fun b c -> f a' b c]
when fun_arity > nargs *)

which instead should use an approach along the lines of:

(* We convert [f a] to [let f’ = f in
let a’ = a in fun b c -> f’ a’ b c]
when fun_arity > nargs *)

The above example illustrates the situation when the effect is delayed indefinitely. For an example, where the effect is delayed until the time of full application, consider:

let k =                                                                                                       
  (let _i = print_int 1                                                                                       
   in fun q -> fun i -> "") ()                                                                                
in k (print_int 0)

which prints 10 when compiled with the bytecode backend and 01 with the native code backend.

The behaviour is the same for version 4.02.3.

Steps to reproduce

$ ocamlc -o func.byte func.ml
$ ./func.byte

$ ocamlopt -o func.native func.ml
$ ./func.native
$

@vicuna
Copy link
Author

vicuna commented May 8, 2017

Comment author: @mshinwell

Sigh, unfortunately the other fixes I'd made about evaluation order haven't fixed this. Thanks for the report. I will make a patch.

@vicuna
Copy link
Author

vicuna commented May 8, 2017

Comment author: @mshinwell

Superceded by #1162

@vicuna vicuna closed this as completed May 8, 2017
@vicuna
Copy link
Author

vicuna commented May 15, 2017

Comment author: @stedolan

It turns out that the same issue crops up with default arguments, although this version can occur with bytecode as well as native code:

let f ?(x = print_endline "hello") () = fun _ -> 1;;

val f : ?x:unit -> unit -> 'a -> int =

f ();;

  • : '_a -> int =

f () ();;

hello

  • : int = 1

@vicuna
Copy link
Author

vicuna commented May 15, 2017

Comment author: @garrigue

With defaults arguments, this is part of the specification (not sure where it is written).
I.e. the evaluation order of default arguments is left unspecified, and can be delayed until after the last functional abstraction.
I.e. side-effects in defaults are only guaranteed to occur before other side effects (in the body of the function).
This is necessary to avoid a code blow-up in native code generation.
This said, it might be sufficient to delay them until after the first non-labeled argument, which would be more intuitive in your example.

@vicuna
Copy link
Author

vicuna commented May 15, 2017

Comment author: @garrigue

About optional arguments: the problem is described in my paper
"Labeled and optional arguments for Objective Caml" of 2001.
See the explanation at the end of section 6.
However, it seems to imply that we should not delay longer than the first non-labeled argument.
I wonder why this is not implemented that way.

@vicuna
Copy link
Author

vicuna commented May 15, 2017

Comment author: @garrigue

Create a pull request which fixes the problem with default arguments.
Need to add some tests...

#1174

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

2 participants