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

sprintf broken when local module named Pervasives is in scope #6417

Closed
vicuna opened this issue May 13, 2014 · 4 comments
Closed

sprintf broken when local module named Pervasives is in scope #6417

vicuna opened this issue May 13, 2014 · 4 comments
Assignees
Labels

Comments

@vicuna
Copy link

vicuna commented May 13, 2014

Original bug ID: 6417
Reporter: Nick Chapman
Assigned to: @garrigue
Status: closed (set by @xavierleroy on 2015-12-11T18:27:41Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.02.0+dev
Category: ~DO NOT USE (was: OCaml general)

Bug description

The following ml is rejected by the latest trunk 4.02
Having a local module named Pervasives in scope, breaks a following sprintf.

module Pervasives = struct end
let _:string = Printf.sprintf "%d" 123

File "bad_sprintf.ml", line 2, characters 30-34:
Error: This expression has type string but an expression was expected of type
('a -> 'b, unit, string) format =
('a -> 'b, unit, string, string, string, string)
CamlinternalFormatBasics.fmt * string

Steps to reproduce

4.02 trunk
commit c82c004

File attachments

@vicuna
Copy link
Author

vicuna commented May 13, 2014

Comment author: @gasche

I'm afraid this is a case that hasn't been considered in the format work. I'll try to think about this.

Could you tell more about your use-case for rebinding Pervasives?

@vicuna
Copy link
Author

vicuna commented May 14, 2014

Comment author: Nick Chapman

The name "Pervasives" is being used as the name for a non top-level module. It is not intended in any way to rebind the real Pervasives.

This was in code which already existed in our code base; but it is not a widely used idiom, and so should be only a small effort to choose a different name.

However, it does seem rather surprising that format strings would compile to reference the definition of Pervasives which is dynamically in scope.

@vicuna
Copy link
Author

vicuna commented May 25, 2014

Comment author: @gasche

I have a first working patch (uploaded above), but it may still need a bit of polish. A full fix is hopefully coming soon.

@vicuna
Copy link
Author

vicuna commented May 27, 2014

Comment author: @garrigue

Fixed by applying formatBasics.diff, committed in 4.02 at revision 14921.

This solves the problem by turning CamlinternalFormatBasics into an independent compilation unit, which is referred to by Pervasives, but does not require it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants