JSAbrahams / mamba

🐍 The Mamba programming language, because we care about safety
MIT License
88 stars 4 forks source link

Add global variable mapping and preserve primitives #416

Closed JSAbrahams closed 1 year ago

JSAbrahams commented 1 year ago

Relevant issues

Summary

Quite a big PR as we had to change some fundamental behaviour. More fundamentally how we deal with constraint generation. Diff slightly larger due to some renaming of variables or consistent change in behaviour, namely how we reference var_mapping.

Multiple constraint sets

We now change the behaviour of the ConstrBuilder such that we always add constraints to multiple sets in parallel. This allows us to generate constraints for all possible execution paths (for instance, after an if or match).

We can then either:

Pushing types

Get rid of unnecessary unions by updating logic of how push_ty operates. Whether one has a union of types is determined there and whether any of the Names is interchangeable.

Tests

Happy

codecov[bot] commented 1 year ago

Codecov Report

Merging #416 (14b36a8) into develop (9723d5f) will decrease coverage by 0.10%. The diff coverage is 95.39%.

:exclamation: Current head 14b36a8 differs from pull request most recent head f497055. Consider uploading reports for the commit f497055 to get more accurate results

@@             Coverage Diff             @@
##           develop     #416      +/-   ##
===========================================
- Coverage    88.43%   88.33%   -0.11%     
===========================================
  Files          110      110              
  Lines        12040    12000      -40     
===========================================
- Hits         10648    10600      -48     
- Misses        1392     1400       +8     
Impacted Files Coverage Δ
src/check/constrain/unify/mod.rs 75.00% <20.00%> (+12.93%) :arrow_up:
src/check/constrain/constraint/iterator.rs 86.20% <66.66%> (-0.46%) :arrow_down:
src/lib.rs 82.89% <75.00%> (+1.19%) :arrow_up:
src/check/result.rs 82.22% <83.33%> (ø)
src/check/constrain/constraint/builder.rs 86.27% <85.71%> (-2.19%) :arrow_down:
src/check/constrain/generate/env.rs 94.44% <96.15%> (-0.56%) :arrow_down:
src/check/constrain/constraint/expected.rs 88.11% <100.00%> (-1.61%) :arrow_down:
src/check/constrain/generate/call.rs 88.20% <100.00%> (+0.10%) :arrow_up:
src/check/constrain/generate/class.rs 82.14% <100.00%> (-6.99%) :arrow_down:
src/check/constrain/generate/collection.rs 96.77% <100.00%> (+0.03%) :arrow_up:
... and 23 more
JSAbrahams commented 1 year ago

Before merging need to document why this works, because the underlying mechanics are starting to become a bit unclear.

JSAbrahams commented 1 year ago

Something went wrong after rebase on develop. So turns out that what we were doing here was actually quite dangerous, could've known.

JSAbrahams commented 1 year ago

It seems that we are now also accidentally allowing any argument, regardless of type, to be passed to functions. Possibly something wrong with how the any boolean in Name affects is_superset_of. See #426

JSAbrahams commented 1 year ago

Must investigate significant drop in coverage first.

One thing which I'm not sure about is what if we shadow a variable with another one of a different type. I say this because I'm not quite sure how the environment and global mapping interact. But I can't think of a definitive way to test this, but a good place to start would be to shadow variables inside a function:

In each test, make sure that we call methods only of that specific class.

JSAbrahams commented 1 year ago

Streamlined quite some logic. After this, implementing dictionary should hopefully be more straightforward.