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
Add hint to "Unbound value .{}" error messages #6765
Comments
Comment author: @gasche I think we should go for the warning-only-for-now approach. Nicer to the user, and I don't think the patch required is large or would add maintenance cost. |
Comment author: @hcarty I would prefer the warning approach as well. |
Comment author: @Octachron I have implemented a rough version of the warning-only-for-now approach. The patch seems rather small and worked on few test cases. |
Comment author: @dbuenzli Also couldn't we have these index operators in a submodule opening |
Comment author: @Octachron I have been tempted for some time to propose a patch to add an Infix/Ops submodule to Bigarray/String/Bytes/Array (and maybe Map and Hashtbl?). For pratical use, it would probably be better if the submodule was included inside the main module, i.e. module Bigarray = struct Has anyone any comment on this idea? |
Comment author: @dbuenzli I don't know if it's useful to do it for the other modules but at least for Bigarrays I think it should be done since it will be the only way to use them and I don't think opening Bigarray should be encouraged (even though the doc says so). I was thinking that maybe we should try to come up with a better name than Ops/Infix for people to systematically use when they have a module which contains only indexing operators. The idea is that if I read that this is being opened I should pay attention to the semantics of .() and .[] in order to figure out whatever imperative crap may be hidden behind this (I personally find this indexing operator redefinition a very dubious idea). I propose |
Comment author: @Octachron
In the current trunk version, the indexing operators are standard operator identifiers, so it is perfectly possible to fetch only these operators from the Bigarray module: let (.{}), (.{}<-) = Bigarray.( (.{}), (.{}<-) );; Having an indexing submodule would be only for convenience reason.
Does it means that you would prefer to have different submodule names for module redefining respectively the .() / .[] / .{} operator family?
I don't see much difference here compared to any other functions or operators. |
Comment author: @Octachron To continue the discussion on an eventual indexing submodule in the bigarray library, I have submitted a pull request here #248. |
Comment author: @alainfrisch If we go for the improved error message only, I have submitted #299 |
Comment author: @alainfrisch Customizable indexing operator won't be in 4.03: postponing this as well. |
Comment author: @mshinwell I have a feeling this could be closed now. |
Comment author: @Octachron I agree that this phantom issue should be closed. |
Original bug ID: 6765
Reporter: @alainfrisch
Assigned to: @Octachron
Status: resolved (set by @Octachron on 2017-03-07T13:37:58Z)
Resolution: fixed
Priority: low
Severity: feature
Version: 4.03.0+dev / +beta1
Category: typing
Tags: patch
Monitored by: @gasche @hcarty @dbuenzli
Bug description
Now that bigarray index operators are defined in Bigarray, one needs to explicitly open this module to be able to use these operators (.{}, .{}<-, and -nary forms). To help in the transition, one could add a special case in the "Unbound value" error printer to suggest opening Bigarray when the name of the unbound value is one of the new operators.
Another approach to simplify porting big code bases would be to add a hack in the type-checker to force looking for these operators in Bigarray instead of failing with Unbound value, and issue a "Deprecated" warning when this is needed.
Ref:
#69
File attachments
The text was updated successfully, but these errors were encountered: