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

result type too generic in pervasives (OCaml 4.03) #6871

Closed
vicuna opened this issue May 14, 2015 · 5 comments
Closed

result type too generic in pervasives (OCaml 4.03) #6871

vicuna opened this issue May 14, 2015 · 5 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented May 14, 2015

Original bug ID: 6871
Reporter: toots
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2016-12-07T10:49:28Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.03.0+dev / +beta1
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: standard library
Parent of: #6872

Bug description

When testing 4.03, I ran into the following warning in a LOT of my files:

Warning 41: Error belongs to several types: exn result
The first one was selected. Please disambiguate if this is wrong.

This is due to the following type, defined in Pervasives:
type ('a,'b) result = Ok of 'a | Error of 'b

Minimal reproduction steps:
bla.ml:
exception Error

let f e =
match e with
| Error -> "foo"
| _ -> "bla"

Compile:
% ocamlopt -w +41 -c bla.ml
File "bla.ml", line 5, characters 7-12:
Warning 41: Error belongs to several types: exn result
The first one was selected. Please disambiguate if this is wrong.

Defining Error exceptions or really anything Error seems quite usual so it would seem natural if Pervasives did not use Error itself..

@vicuna
Copy link
Author

vicuna commented May 14, 2015

Comment author: toots

Actually the result type does not even seem to be used in Pervasives..

@vicuna
Copy link
Author

vicuna commented May 15, 2015

Comment author: @yallop

There's some discussion of the result type on the pull request that introduced it:

#147

@vicuna
Copy link
Author

vicuna commented May 15, 2015

Comment author: toots

Thanks, I've responded there. I really hope that this is fixed for the final release, this Error squatting isn't cool to me..

@vicuna
Copy link
Author

vicuna commented May 15, 2015

Comment author: @gasche

Looking at actual examples indicates that we cannot properly estimate the invasiveness of this new constructor before #6872 is settled.

@vicuna
Copy link
Author

vicuna commented May 24, 2015

Comment author: @gasche

According to the discussion in
#147
only a few warnings remain after #6872 is fixed, which seems acceptable.

I'm thus marking this issue as resolved.

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