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

feat: register MAX and MIN macros for stdint types #1412

Merged
merged 5 commits into from
Apr 13, 2022

Conversation

scolsen
Copy link
Contributor

@scolsen scolsen commented Apr 7, 2022

Adds MAX and MIN for each INT type and MAX for each UINT type.
These are macros defined by stdint.h and are sometimes useful for bounds
determinations and conversions and such.

Adds MAX and MIN for each INT<N> type and MAX for each UINT<N> type.
These are macros defined by stdint.h and are sometimes useful for bounds
determinations and conversions and such.
@scolsen scolsen requested a review from a team April 7, 2022 02:03
Copy link
Collaborator

@eriksvedang eriksvedang 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! Should these be interfaces?

@scolsen
Copy link
Contributor Author

scolsen commented Apr 7, 2022

Looks good! Should these be interfaces?

Good point! They probably should since we have MAXes for the other integer types too. I can make the change.

Since several numeric types define maximum and minimum values, it makes
sense for these to be defined as interfaces. This commit also makes
existing definitions of MAX and MIN for Carp's numeric types implement
the interfaces.
@scolsen
Copy link
Contributor Author

scolsen commented Apr 9, 2022

Added interfaces!

@scolsen scolsen requested a review from eriksvedang April 9, 2022 16:13
* fix: respect let binding shadowing in memory management

Previously, we didn't account for shadowing in let bindings in our
memory management routines. This led to rare situations in which
multiple deleters might be added for a single variable name, for
example:

```clojure
(defn n [xs]
  (let [xs [1 2 3]
        n &xs]
    n))
```

The borrow checker would fail on this code since it would assign `xs`
two deleters, one for the untyped argument and another for the let
binding.

Instead, we now perform *exclusive* ownership transfer for the duration
of the let scope--when a shadow is introduced, the previous deleters for
that variable name are dropped until the end of the let scope, since we
evaluate all instances of the shadowed name to the more local binding.
At the end of the let scope, the original deleter is restored.

Fixes issue carp-lang#597

* refactor: improved dead reference error for let

Since let scopes resolve to their bodies, we can report the body of the
let as the xobj producing an error when a dead reference is returned.

* test: update error message for dead refs in let

* test: add regression test for issue carp-lang#597

Ensure we don't regress and fail to manage memory when let bindings
shadow function argument names.
@scolsen
Copy link
Contributor Author

scolsen commented Apr 11, 2022

so it turns out these interfaces are breaking tests because our interface resolution currently overwrites symbol lookup modes! I've got a fix.

@scolsen
Copy link
Contributor Author

scolsen commented Apr 11, 2022

Once #1415 merges the tests will pass again and we can merge this.

* fix: respect symbol modes on interface concretization

When concretizing interfaces (finding the appropriate implementation at
a call site) we previously set the lookup mode of all such resolved
symbols to CarpLand AFunction. This incorrectly overwrites the lookup
mode of Externally registered types, causing them to emit incorrect C
when the user specifies an override.

We now preserve whatever lookup mode is assigned to the implementation
the concretization resolves the interface to. This not only fixes the
external override emission issue, but should be more correct in general.

fixes carp-lang#1414

* test: add regression test for issue carp-lang#1414
@eriksvedang eriksvedang merged commit 5d530b7 into carp-lang:master Apr 13, 2022
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.

None yet

2 participants