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

lazy types get displayed in odd and inconsistent ways #6669

Closed
vicuna opened this issue Nov 22, 2014 · 11 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Nov 22, 2014

Original bug ID: 6669
Reporter: TimLeonard
Assigned to: @lpw25
Status: closed (set by @xavierleroy on 2016-12-07T10:37:03Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: MacBook Air
OS: OS X
OS Version: 10.9
Version: 4.02.1
Category: typing
Related to: #6684

Bug description

A value of type may be displayed by the interactive top level in any of several forms, depending on how it's constructed:
val form1 : string Lazy.t = lazy "xyz"
val form2 : string Lazy.t =
val form3 : string lazy_t = lazy

The first form is good. The other two are unhelpful.

Steps to reproduce

type record = { text : string Lazy.t; };; (* Define a record type to carry some lazy text. )
let make_record s = { text = lazy s; };; (
Define a function to create a value of that type. )
let lazily_concat records = (
Define a function to lazily concatenate the texts carried by a list of such records. )
{ text = lazy ( let force_text record = Lazy.force record.text in
let forced_texts = List.map force_text records in
(String.concat "" forced_texts)
)
};;
let form1 = (make_record "xyz").text;; (
Display a value of lazy text. )
Lazy.force form1;;
let form2 = (lazily_concat [make_record "xyz"]).text;; (
Display the same value constructed a different way. )
Lazy.force form2;;
let form3 = lazy "xyz";; (
And again, a third way. *)
Lazy.force form3;;

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @gasche

It's not a bug, it's a feature.

Displaying lazy values is a game of guessing: you don't want to force the evaluation of the lazy thunk, so they will not in general be able to display anything meaningful. It's the you've seen:

let test = lazy ("foo" ^ "bar");;

val test : string lazy_t =

lazy ("foo" ^ raise Exit);;

  • : string lazy_t =

Once a lazy value has been forced, the printer will notice it and provide more information about the content:

ignore (Lazy.force test);;

test;;

  • : string lazy_t = lazy "foobar"

Finally, in some cases the compiler is able to recognize that the expression does not need to be thunked in the first place, as it is an already-evaluated value, so it will create it in "forced state" from the start.

lazy "foobar";;

  • : string lazy_t = lazy "foobar"

This is all good. If you wanted the same printing each time, you'd get each time, which is less useful than the current mode.

(The difference between Lazy.t and lazy_t is inconsequential, as both types are equal. If you want a more canonical printing of type paths, use the -short-paths option available since 4.01.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @lpw25

The first two forms are correct, but I think that the third form is a bug in the new cycle detection code. So this issue should probably be reopened.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @gasche

Ah, I had only reproduced under 4.01 so I missed the regression, sorry.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @lpw25

The attached patch fixes it.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @gasche

I'm not convinced by the patch as-is. First, it changes the code structure which was here before the cycle-printing change (and I don't understand why that is necessary); second, it behaves differently on forward-tag value and other non-thunk values, while my understanding was that the GC can change forward tags into other non-thunk at will (Why would the printing behavior depend on this? I think I understand that forward-tag means Lazy.force returns a physically distinct block, and thus the false-cycle bug does not happen in this case, but the special-casing of the code here still feels wrong.).

Could you explain what goes wrong if you simply remove the "nest" call on is_lazy_val arguments? (In the final patch, a comment about this pitfall would be nice.)

Maybe we could add some cycle-avoidance logic to the nest_gen function: optionally pass an extra value with the semantics of "do not count it as a cycle if it is this already-seen value (or forwards to it?)".

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @lpw25

There is a corner case:

let rec x = lazy x

which justifies the parts of the patch which worry you.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @gasche

I uploaded a commented patch following your suggestions. Do you have any feedback on it?

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 22, 2014

Comment author: @lpw25

The comments are accurate and the code is correct. I would inline the code from maybe_nest because having it as a function just obfuscates things.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 23, 2014

Comment author: @gasche

fixed in 4.02 and trunk.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 29, 2014

Comment author: @gasche

Leo: because of an unrelated issue, in #6684 ( #6684 ) I'm considering reverting to an implementation that is, ironically, much closer to your first patch proposal (reasoning on the tag directly instead of relying on Lazy.is_val). I also make changes to the nest_gen function, trying to remove uses of the Obj module and use the functor-argument O instead. I would be interested in your opinion on these patches.

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 4, 2014

Comment author: @lpw25

Sorry for the slow reply. The new patch looks good to me.

There is a slight inaccuracy in the comments though. The representation you refer to as case "3" doesn't only come from creating a lazy directly from a value, the garbage collector also rewrites forward pointers (case "2" in your description) into this form.

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.