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

Qualify () constructor #418

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Qualify () constructor #418

merged 1 commit into from
Apr 17, 2023

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Feb 23, 2023

We are building a hidden expression that contains "()" but it is not
qualified. So it will pick the constructor in scope. This can cause
problems if () has been redefined. The correct fix is to qualify it as
part of the Unit module.

(additionally, this removes an unused ident)

Fixes #417

We are building a hidden expression that contains "()" but it is not
qualified. So it will pick the constructor in scope. This can cause
problems if `()` has been redefined. The correct fix is to qualify it as
part of the `Unit` module.

(additionally, this removes an unused ident)

Fixes ocaml-community#417
@emillon emillon merged commit ed89528 into ocaml-community:master Apr 17, 2023
emillon added a commit to emillon/opam-repository that referenced this pull request Apr 17, 2023
CHANGES:

* Add support for OCaml 5.1 (ocaml-community/utop#421, @emillon)

* Mark `prompt_continue`, `prompt_comment`, `smart_accept`, `new_prompt_hooks`,
  `at_new_prompt` as deprecated (they have been documented as such since 2012
  and most of them are ignored) (ocaml-community/utop#415, @emillon)

* Qualify `()` constructor in generated expressions. (ocaml-community/utop#418, fixes ocaml-community/utop#417, @emillon)
@Octachron
Copy link
Member

An issue with this change just appeared: the core library redefine the Unit module but without re-exporting () thus:

open Core;;
1 + 1;;

fails with

Error: Unbound constructor Unit.()

emillon added a commit to emillon/utop that referenced this pull request Apr 21, 2023
Fixes ocaml-community#428

The change in ocaml-community#418 introduced a regression if a `Unit` module in scope
does not have a `()` constructor in it.

This switches to the more explicit `Stdlib.Unit.()` path.
emillon added a commit to emillon/utop that referenced this pull request Apr 21, 2023
Fixes ocaml-community#428

The change in ocaml-community#418 introduced a regression if a `Unit` module in scope
does not have a `()` constructor in it.

This switches to the more explicit `Stdlib.Unit.()` path.
emillon added a commit that referenced this pull request Apr 21, 2023
Fixes #428

The change in #418 introduced a regression if a `Unit` module in scope
does not have a `()` constructor in it.

This switches to the more explicit `Stdlib.Unit.()` path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redefining constructor () with an argument breaks everything
2 participants