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

Update creation of initial environment #1052

Merged
merged 5 commits into from
Nov 26, 2019
Merged

Update creation of initial environment #1052

merged 5 commits into from
Nov 26, 2019

Conversation

trefis
Copy link
Contributor

@trefis trefis commented Nov 8, 2019

Merlin and the compiler behaved differently since 4.07 (and ocaml/ocaml#1010) (cf. #1003).
This PR fixes that, and also updates 4.08 to use the new Load_path module from the compiler distribution.

While I'm confident about the 4.07 fix, the 4.08 might degrade performances, I'll be monitoring that at janestreet over the next few weeks before merging this PR.

@let-def let-def mentioned this pull request Nov 11, 2019
(* Mapping from basenames to full filenames *)
type registry = string SMap.t ref

let state = Local_store.new_bindings ()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The state value represents a list of dynamically-scoped variables.
With srefk create new variables, but at no point in time you redefine these variables.
See https://github.com/ocaml/merlin/blob/master/src/kernel/mocaml.ml#L30 for instance.

The original design assumed that there might be different notion of global scopes, hence the distinction between Btype.state and Env.state.
However I think a single typechecker_state might be sufficient, and so this notion of binding could be removed.

I will write a PR to simplify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I rebased and this should now be fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the last commit (which uses Local_store.Typechecker) breaks the tests.
I guess there's something I missunderstood.

src/ocaml/typing/408/typemod.ml Outdated Show resolved Hide resolved
This pulls in the new Load_path module from the compiler distribution,
but adds a layer of caching on directories content.
@trefis trefis requested a review from let-def November 11, 2019 11:04
@trefis trefis merged commit 98b9457 into master Nov 26, 2019
let-def added a commit to let-def/opam-repository that referenced this pull request Nov 29, 2019
CHANGES:

Fri Nov 29 17:35:58 CET 2019

  + backend
    - support OCaml 4.09 (ocaml/merlin#1055)
    - fix parse errors in 4.08 (ocaml/merlin#1037)
    - update 4.08 support to OCaml 4.08.1 (ocaml/merlin#1053)
    - support `without_cmis`
    - separate reading from caching in file-cache, use caching in
      `Env.check_state_consistency` (ocaml/merlin#1044)
    - simplify compiler state management (ocaml/merlin#1056, ocaml/merlin#1059)
    - fix creation of initial environment, improve compatibility with
      upstream 4.08 (ocaml/merlin#1052)
  + frontend
    - code re-organization (ocaml/merlin#1042)
    - error command: select which kind of errors to show (ocaml/merlin#995)
    - print value types in outline (ocaml/merlin#1014)
    - fix process handling in windows (ocaml/merlin#1005)
  + editor modes
    - emacs
      + bugfixes in merlin-imenu, merlin-xref (ocaml/merlin#1000, ocaml/merlin#1021, ocaml/merlin#1001)
      + show types in merlin-imenu (ocaml/merlin#1013)
      + reset buffer local configurations when resetting server (ocaml/merlin#1004)
      + remove merlin-use-tuareg-imenu
      + fix stack overflow (ocaml/merlin#1024)
      + fix merlin-occurrence (ocaml/merlin#1043)
    - vim
      + display warn-error warnings as errors (ocaml/merlin#1009)
  + testsuite
    - cover file-cache and `check_state_consistency` (ocaml/merlin#1044)
    - check inconsistent assumptions, test server versus single modes (ocaml/merlin#1047)
let-def added a commit to let-def/opam-repository that referenced this pull request Mar 2, 2020
CHANGES:

Fri Nov 29 17:35:58 CET 2019

  + backend
    - support OCaml 4.09 (ocaml/merlin#1055)
    - fix parse errors in 4.08 (ocaml/merlin#1037)
    - update 4.08 support to OCaml 4.08.1 (ocaml/merlin#1053)
    - support `without_cmis`
    - separate reading from caching in file-cache, use caching in
      `Env.check_state_consistency` (ocaml/merlin#1044)
    - simplify compiler state management (ocaml/merlin#1056, ocaml/merlin#1059)
    - fix creation of initial environment, improve compatibility with
      upstream 4.08 (ocaml/merlin#1052)
  + frontend
    - code re-organization (ocaml/merlin#1042)
    - error command: select which kind of errors to show (ocaml/merlin#995)
    - print value types in outline (ocaml/merlin#1014)
    - fix process handling in windows (ocaml/merlin#1005)
  + editor modes
    - emacs
      + bugfixes in merlin-imenu, merlin-xref (ocaml/merlin#1000, ocaml/merlin#1021, ocaml/merlin#1001)
      + show types in merlin-imenu (ocaml/merlin#1013)
      + reset buffer local configurations when resetting server (ocaml/merlin#1004)
      + remove merlin-use-tuareg-imenu
      + fix stack overflow (ocaml/merlin#1024)
      + fix merlin-occurrence (ocaml/merlin#1043)
    - vim
      + display warn-error warnings as errors (ocaml/merlin#1009)
  + testsuite
    - cover file-cache and `check_state_consistency` (ocaml/merlin#1044)
    - check inconsistent assumptions, test server versus single modes (ocaml/merlin#1047)
@trefis trefis deleted the initial_env branch January 29, 2021 14:38
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.

2 participants