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

Use single typevar map for entire spec #2629

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Conversation

thpani
Copy link
Collaborator

@thpani thpani commented Jul 3, 2023

Use a single type converter for an entire quint spec.

We previously used one type converter per sub-expression. This re-initialized the map for translating Quint type variables (named) to Apalache TypeSystem 1 type variables (numbered), causing wrong type annotations. For example the type of the following Quint was translated as (a,a) => {src: a, ts: a}.

/// `p.msgFrom(ts)` is a message sent from `p` at time `ts` 
def msgFrom(p, ts) = {src: p, ts : ts}
  • Tests added for any new code
  • Ran make fmt-fix (or had formatting run automatically on all files edited)
  • Documentation added for any new functionality
  • Entries added to ./unreleased/ for any new functionality

@thpani thpani requested a review from konnov July 3, 2023 12:04
@thpani thpani requested a review from Kukovec as a code owner July 3, 2023 12:04
@thpani thpani self-assigned this Jul 3, 2023
@thpani thpani requested a review from shonfeder as a code owner July 3, 2023 12:04
@thpani thpani force-pushed the th/one-typevar-map branch 2 times, most recently from 5fe1cee to 2818928 Compare July 3, 2023 12:21
@codecov-commenter
Copy link

Codecov Report

Merging #2629 (2818928) into th/refactor-quint (fddea31) will decrease coverage by 0.01%.
The diff coverage is 92.30%.

@@                  Coverage Diff                  @@
##           th/refactor-quint    #2629      +/-   ##
=====================================================
- Coverage              78.49%   78.49%   -0.01%     
=====================================================
  Files                    447      447              
  Lines                  15654    15653       -1     
  Branches                2553     2544       -9     
=====================================================
- Hits                   12288    12287       -1     
  Misses                  3366     3366              
Impacted Files Coverage Δ
...forsyte/apalache/io/quint/QuintTypeConverter.scala 97.29% <ø> (-0.08%) ⬇️
...ain/scala/at/forsyte/apalache/io/quint/Quint.scala 85.22% <92.30%> (+0.05%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@konnov konnov left a comment

Choose a reason for hiding this comment

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

Nice!

* QuintTypeConverter.getVarNo() for details).
*/
object QuintTypeConverter {
def apply(quintType: QuintType): TlaType1 = new QuintTypeConverter().convert(quintType)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch! I can see how the companion object was creating a new instance every time

@thpani thpani merged commit a04df6d into th/refactor-quint Jul 4, 2023
10 checks passed
@thpani thpani deleted the th/one-typevar-map branch July 4, 2023 08:57
@thpani thpani mentioned this pull request Jul 4, 2023
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.

3 participants