Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Exception tracking #666

Merged
merged 28 commits into from
Mar 1, 2022
Merged

Exception tracking #666

merged 28 commits into from
Mar 1, 2022

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Feb 10, 2022

This PR copies exception tracking from membrane into semantic-analysis, transposing as necessary to reflect the differing domain abstractions. (This was intended to have been done at the same time as #659, but got overlooked.)

Note that some details necessarily differ slightly; our Dom abstraction is a fair bit different, and in particular their ddie (which throws an exception) takes a String, while ours takes a val (as of #664), making ours more expressive, but more subtle as well.

I'm likely to push the modular abstract interpretation stuff to a future PR.

@robrix robrix marked this pull request as ready for review February 25, 2022 16:35
@robrix robrix requested a review from a team as a code owner February 25, 2022 16:35
Copy link
Contributor Author

@robrix robrix left a comment

Choose a reason for hiding this comment

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

🤘🏻😈🤘🏻

@@ -0,0 +1,40 @@
-- GHCI settings, collected by running cabal repl -v and checking out the flags cabal passes to ghc.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and several other files exist solely to make local development of just semantic-analysis more convenient.

Comment on lines -67 to -70
data Snoc a = Nil | Snoc a :> a
deriving (Foldable, Functor, Traversable)


Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looked like it would be important for Snoc to be accessible elsewhere. It turned out not to be, but there's no reason it shouldn't have its own home.

Comment on lines -74 to +72
dif :: Has (Dom val) sig m => val -> m a -> m a -> m a
dif :: Has (Dom val) sig m => val -> m val -> m val -> m val
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary for us to collect the results from both branches of a conditional in exception tracking. a would always be val during abstract interpretation anyway, so it doesn't hurt us any.

Comment on lines +54 to +55
DAbs _ b -> runExcC (hdl (b mempty <$ ctx))
DApp f a -> pure $ f <> a <$ ctx
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We dive into abstractions eagerly, like a typechecker would. Then in applications we can simply union up the results.

This is wrong: it means we'll charge exceptions thrown within a lambda to the lambda, not to call sites, as tho the exceptions were thrown when constructing the lambda, rather than when calling it. Wrong, but convenient: it means we don't have to construct closures (or any model thereof) and then have to deal with partiality on application.

DBool _ -> pure nil
DIf c t e -> fmap (mappend c) <$> runExcC (hdl (t <$ ctx) <|> hdl (e <$ ctx))
DString _ -> pure nil
DDie e -> pure $ e <> fromExceptions [Exception n | n <- Set.toList (freeVariables e)] <$ ctx
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only really interesting case. We return all the free variables and exceptions in the argument, as well as making exceptions from the former.

That means raise IOError is going to (correctly) throw IOError, while raise "foo" won't return anything (underapproximation), and raise Foo(bar) will throw both Foo and bar (overapproximation).

patrickt
patrickt previously approved these changes Mar 1, 2022
Copy link
Contributor

@patrickt patrickt 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, though I have a couple questions/nitpicks.

🎩 @patrickt for an excellent observation.
Copy link
Contributor

@patrickt patrickt left a comment

Choose a reason for hiding this comment

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

PR #666… you’re truly doing the devil’s work! 😈

@robrix robrix merged commit 4f6a999 into master Mar 1, 2022
@robrix robrix deleted the exceptional-tracing branch March 1, 2022 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants