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

Merge updated_enabled_cdot. #148

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open

Merge updated_enabled_cdot. #148

wants to merge 36 commits into from

Conversation

kape1395
Copy link
Collaborator

Here is an attempt to merge old branch updated_enabled_cdot.

johnyf and others added 30 commits March 31, 2021 14:24
Otherwise the already computed weights of an operator can
rely on the arity of the operator before the dependence on
parameters of the definition (`Foo(x) == INSTANCE Inner`)
is added to the instantiated operator signature.

Parametric instantiation changes the arity of operators,
so the weights list needs to be recomputed, based on the
arity after instantiation has been performed.
This change ensures that the testing commands
are read after the closing horizontal rule (`=====*`)
of the module.

To do this, the nesting depth of submodules is tracked,
by incrementing the counter `submodule_nesting_depth` when
encountering a module opening (`-----*\s*MODULE`), and
decrementing this counter when encountering
a module closing (`=====*`).

Commands are read after the module's end.
Previously, commands were read after the first module closing
(`=====*`), which results in errors in the presence
of submodules.
that prints the names of modules in
a module context.
When instantiating inside a LET, definitions are kept,
and other module units omitted. A negative shift is
applied to the remaining module units. This negative shift
should equal minus the number of hypotheses that the
omitted module unit introduces in the context.

The number of hypotheses for each module unit is
computed by the function `M_t.hyps_of_modunit`.
`Module.Elab` replaces each `INSTANCE` statement
with definitions. If the instantiated module
extends the module `TLAPS`, then the definitions
include backend pragmas (constructor `Bpragma`).
`Module.Elab` replaces each `INSTANCE` statement
with definitions. If the instantiated module
extends the module `TLAPS`, then the definitions
include backend pragmas (constructor `Bpragma`).
This change makes fingerprints take into account
the result of the proof directive AutoUSE.

Previously, fingerprinting was done before any
expansion of definitions, normalization, and
automated expansion of definitions, expansion of
ENABLED and of \cdot.

This approach worked correctly for BY DEF definitions,
because those definitions were included in the
fingerprint as context.

With this change, only for proof obligations that
include the proof directive AutoUSE, the fingerprint
is computed after expansion of definitions, normalization,
automated expansion of definitions, expansion of ENABLED,
and of \cdot. This change ensures that the proof
obligation is fingerprinted with all automatic
expansions of definitions applied.

A test is added that catches this error.
summary of changes

- test fingerprints by running `tlapm` twice
- add new proof directive `ENABLEDrules`
- rewrite `ENABLEDaxioms` to remove the
  Boolean typeness assumptions
- rewrite two proof rules that previously were in
  `ENABLEDaxioms` and now are in `ENABLEDrules`,
  to remove the Boolean typeness assumptions
- correct soundness check for `ENABLEDrules`
  (previously for `ENABLEDaxioms`)
- fingerprint the level correctness of proof obligations
- revise renaming of variables in `ExpandENABLED`

- removal of older implementation of `ENABLEDaxioms`
- `ENABLEDaxioms` was not correctly collecting primed variables
- use expression level in fingerprint of definition
- record name of `Bpragma` in fingerprint
- implement rewriting system for `ENABLEDrewrites`
- add new proof directive `ENABLEDrewrites`
- add test modules
These changes are for now kept as a separate
commit, in case further rebasing is needed
before preparing the pull request for the
branch `update_enabled_cdot`.
Signed-off-by: Karolis Petrauskas <[email protected]>
@kape1395
Copy link
Collaborator Author

kape1395 commented Sep 1, 2024

The LSP server currently fails with

LSP request failed with exception Not_found, backtrace:
Raised at Stdlib__List.find in file "list.ml", line 232, characters 10-25
Called from Tlapm_lib__Property.get in file "src/util/property.ml", line 71, characters 9-54
Called from Tlapm_lib__E_levels.get_level_info in file "src/expr/e_levels.ml" (inlined), line 68, characters 23-47
Called from Tlapm_lib__E_levels.get_level in file "src/expr/e_levels.ml", line 70, characters 21-37
Called from Tlapm_lib__Fingerprints.fp_sequent.spin in file "src/backend/fingerprints.ml", line 446, characters 23-48
Called from Tlapm_lib__Fingerprints.fp_sequent.spin in file "src/backend/fingerprints.ml", line 441, characters 13-26
...

It seems like the levels are not assigned somewhere.

@kape1395 kape1395 self-assigned this Sep 1, 2024
@kape1395 kape1395 marked this pull request as ready for review September 1, 2024 21:11
@kape1395
Copy link
Collaborator Author

kape1395 commented Sep 1, 2024

@muenchnerkindl, @damiendoligez, who could review this PR?

Copy link
Contributor

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

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

Looks good modulo a few suggestions.

(***************************************************************************)
Inv ==
\/ P0:: \A i \in Nodes : tpos < i => ~ active[i]
\/ P1:: \E j \in 0 .. tpos : color[j] = "black"
\/ P2:: tcolor = "black"

-----------------------------------------------------------------------------
USE NAssumption

(* TypeOK is an inductive invariant *)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(* TypeOK is an inductive invariant *)
(* TypeCorrect is an inductive invariant *)

@@ -351,7 +351,8 @@ module Make (CB : Callbacks) = struct
with exc ->
let open Jsonrpc.Response.Error in
let exc_str = Printexc.to_string exc in
Eio.traceln "LSP request failed with exception %s" exc_str;
let exc_btr = Printexc.get_backtrace () in
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to get the backtrace first, before Printexc.to_string has a chance to change it, so swap this line with the previous one.

"Level<=1"
else
"Level>1"
end else "" in
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
end else "" in
end
else "" in

Comment on lines +62 to +63
| ENABLEDrewrites
| ENABLEDrules
Copy link
Contributor

Choose a reason for hiding this comment

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

This change makes fingerprint files incompatible with previous versions of TLAPM, so we'll have to change the magic number. I've prepared a pull request on this pull request: #176

Comment on lines +1452 to +1453
if (not (Hashtbl.mem primed_vars var_name)) then
Hashtbl.add primed_vars var_name ()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (not (Hashtbl.mem primed_vars var_name)) then
Hashtbl.add primed_vars var_name ()
Hashtbl.replace primed_vars var_name ()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants