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

Lint: Node pattern match uses sub-type instead of type equality. #79

Merged
merged 4 commits into from
Mar 3, 2020

Conversation

andorp
Copy link
Member

@andorp andorp commented Feb 22, 2020

No description provided.

Copy link
Member

@csabahruska csabahruska left a comment

Choose a reason for hiding this comment

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

ok

Copy link
Member

@Anabra Anabra left a comment

Choose a reason for hiding this comment

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

Nice change! I always wanted to handle all tose false negatives :)

Comment on lines +227 to 237
it "doesn't alert over-approximated binds" $ do
let program = [prog|
main =
n0 <- pure (CInt 0)
n1 <- case n0 of
(CInt c0) -> pure n0
(CFloat c1) ->
a0 <- pure (CFloat 2.0)
pure a0
i <- pure (CInt 1)
f <- pure (CFloat 1.0)
l <- store i
update l f
v <- fetch l
(CFloat f2) <- pure v
pure ()
|]
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this semantically invalid GRIN code? As in store, fetch and update must always come in this order for a given location.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure. We could simply insert a fetch operation before the update, and that wouldn't change the result of the HPT.
@csabahruska What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

IMO it is not the point of this specific test so it is acceptable here.

Comment on lines 242 to 252
it "disregards variable patterns" $ do
let program = [prog|
main =
n0 <- case 0 of
0 ->
n1 <- pure (CInt 0)
pure n1
1 ->
n2 <- pure (CFloat 0.0)
pure n2
(CInt x) <- case n0 of
n0 <- pure (CInt 0)
n1 <- case n0 of
(CInt c0) -> pure n0
(CFloat c1) ->
a0 <- pure (CInt 0)
a0 <- pure (CFloat 2.0)
pure a0
pure ()
|]
Copy link
Member

Choose a reason for hiding this comment

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

Could you please name all subexpressions, so that adopting these tests to the new syntax will be easier?

grin/test/LintSpec.hs Outdated Show resolved Hide resolved
grin/test/LintSpec.hs Outdated Show resolved Hide resolved
grin/src/Grin/Lint.hs Outdated Show resolved Hide resolved
andorp and others added 3 commits March 3, 2020 21:16
Co-Authored-By: Peter Podlovics <peter.d.podlovics@gmail.com>
Co-Authored-By: Peter Podlovics <peter.d.podlovics@gmail.com>
Co-Authored-By: Peter Podlovics <peter.d.podlovics@gmail.com>
@Anabra Anabra merged commit bc14718 into master Mar 3, 2020
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