Rework global state #1059
Merged
Rework global state #1059
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
trefis
reviewed
Nov 20, 2019
src/utils/local_store.ml
Outdated
|
|
||
| let new_bindings () = | ||
| { refs = [] } | ||
| let is_bound = ref false in | ||
| { refs = [F (is_bound, (fun () -> true))]; is_bound } |
There was a problem hiding this comment.
reset (new_bindings ()) sets is_bound to true. I don't understand why.
trefis
reviewed
Nov 20, 2019
src/kernel/mocaml.ml
Outdated
|
|
||
| type typer_state = Local_store.scope | ||
|
|
||
| let bound = srefk None |
There was a problem hiding this comment.
I think the code would be slightly easier to read if this was named current_state.
trefis
reviewed
Nov 21, 2019
| @@ -29,7 +29,7 @@ module Cache = struct | |||
| in | |||
| match List.assoc key !cache with | |||
| | state -> | |||
| cache := List.remove_assoc key !cache; | |||
| cache := (key, state) :: List.remove_assoc key !cache; | |||
There was a problem hiding this comment.
The only reason I can see for doing this is "it's going to be quicker to access next time".
If that's the case, let's perhaps put a comment saying that before the line.
But also: the list seems be limited to a length of 6; is the change really worth it?
There was a problem hiding this comment.
It's a quick'n'dirty implementation of a LRU (least recently used) cache: we cache the last 6 recently used instances of the typechecker.
If we didn't move the state to the head of the list, that would implement another caching policy... 🤷♂️
|
I implemented the changes you suggested. I also added a check that we don't create new references after an instance has been created. |
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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the way state from the compiler is managed.
It now mimics a much simpler form of dynamic-scoping were bindings cannot be nested and there is a unique "binding point" that snapshots the whole compiler state.
If this doesn't break the cache (in correctness nor performance), it will help further changes (path management that @trefis worked on) and can be the basis of a hypothetical upstreaming of state management.