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

Analysis effects #379

Merged
merged 271 commits into from
Jan 14, 2020
Merged

Analysis effects #379

merged 271 commits into from
Jan 14, 2020

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Nov 7, 2019

This PR replaces the Analysis record type with effects & carriers modelling the environment, heap, and domain.

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.

Ready for review!

:def! no-pretty \ _ -> pure ":set -interactive-print System.IO.print"
:def! r \_ -> pure ":reload\n:pretty"
:set -package-id prtty-smpl-3.1.1.0-c89f0500
:set -interactive-print Text.Pretty.Simple.pPrint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

semantic-ast depends on pretty-simple now, which makes it much more challenging to use -package pretty-simple in your ~/.ghci file since ghci “helpfully” ends up loading it twice.

You can avoid that issue by using -package-id, at the cost of having to keep that up to date whenever semantic gets an updated version of it.

I’m not happy with this, but I don’t know a better means of doing this aside from e.g. introducing a new registered package which nothing in semantic depends on just to re-export pretty-simple’s interface, or using a different pretty-printer in semantic than used in the REPL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes! We should probably submit a bug somewhere to somebody?

@@ -183,7 +183,7 @@ steps:
# between actual import and closing bracket.
#
# Default: true
align: true
align: false
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 formats LANGUAGE pragmas each on a new line, with the #-} markers unaligned (for diff minimality).

{-# LANGUAGE DeriveFunctor #-}
{-# LANGUAGE DeriveGeneric #-}

Comment on lines -13 to -15
# do a build of dependencies up front to ensure they’re all available
cabal v2-build -v0 all --only-dependencies

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 didn’t work out as well as I’d hoped—the silent build means that you can get ghcide and/or script/repl appearing to just hang while it deals with building dependencies.

I’ve moved this to script/repl and removed the -v0.

@@ -43,6 +40,7 @@ function flags {
echo "-isemantic-java/src"
echo "-isemantic-json/src"
echo "-isemantic-python/src"
echo "-isemantic-python/test"
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 lets script/repl find the source files in semantic-python’s tests.

Comment on lines +9 to +10
# do a build of dependencies up front to ensure they’re all available
cabal v2-build all --enable-benchmarks --enable-tests --only-dependencies
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In addition to moving this back here and removing the -v0 (which silences all output), this now also ensures that the benchmarks’ & tests’ dependencies have been built before loading the repl.

Comment on lines -2 to -4
module Analysis.Analysis
( Analysis(..)
) where
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Success: this module no longer exists!

Comment on lines +2 to +7
module Analysis.Carrier.Heap.Monovariant
( -- * Heap carrier
HeapC(..)
-- * Heap effect
, module Analysis.Effect.Heap
) where
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Factoring this out means that the abstract analyses get to share a lot of code.

Comment on lines -1 to +16
{-# LANGUAGE DerivingVia, FlexibleContexts, FlexibleInstances, LambdaCase, MultiParamTypeClasses, NamedFieldPuns,
OverloadedStrings, RankNTypes, RecordWildCards, ScopedTypeVariables, TypeApplications, TypeOperators,
UndecidableInstances #-}
{-# LANGUAGE DerivingVia #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE QuantifiedConstraints #-}
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE RecordWildCards #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE StandaloneDeriving #-}
{-# LANGUAGE TypeApplications #-}
{-# LANGUAGE TypeOperators #-}
{-# LANGUAGE UndecidableInstances #-}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An example of the new LANGUAGE pragma style.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don’t mind this (though I would encourage everyone to turn on automatic execution of stylish-haskell on save, so we can start migrating to this style without murdering history with whitespace changes).

data Concrete term name
= Closure Path.AbsRelFile Span name (term name) (Env name)
data Concrete term
= Closure Path.AbsRelFile Span (Named (Scope () term Addr))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By holding a scope parameterized by Addr, Closure obviates the need for the Addr parameter it formerly required.

-> File (term name)
-> m (File (Either (Path.AbsRelFile, Span, String) (Concrete term name)))
-> File (term Addr)
-> m (File (Either (Path.AbsRelFile, Span, String) (Concrete term)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of parameterizing the incoming terms by some arbitrary name type, we parameterize them by the address type for the analysis in question.

In practice this means that we can only analyze closed terms, since you don’t in general have a way to place addresses in the syntax—there’s no way to allocate them from outside the analysis.

My plan is to deal with this by binding free names—names defined in other modules, for example—with existentials, which we will allocate addresses for and can instantiate however we wish. (This is essentially a more principled but otherwise equivalent approach to holes as used in Evaluatable.) Given an input term with existentials, we should ideally produce a result defined modulo the unknown information which we can then supply to resolve the result after the fact. This will allow us to compute partial & incremental analyses of input programs without requiring configuration.

@robrix robrix marked this pull request as ready for review January 6, 2020 22:29
@robrix robrix requested a review from a team January 7, 2020 16:03
:def! no-pretty \ _ -> pure ":set -interactive-print System.IO.print"
:def! r \_ -> pure ":reload\n:pretty"
:set -package-id prtty-smpl-3.1.1.0-c89f0500
:set -interactive-print Text.Pretty.Simple.pPrint
Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes! We should probably submit a bug somewhere to somebody?

)
=> Algebra (A.Domain term Addr (Concrete term) :+: sig) (DomainC term m) where
alg = \case
L (L (A.Unit k)) -> k Unit
Copy link
Contributor

Choose a reason for hiding this comment

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

These L and R pattern matches are a real doozy.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don’t see what could be done about them, though.

@robrix robrix merged commit 87d526f into master Jan 14, 2020
@robrix robrix deleted the analysis-effects branch January 14, 2020 12:27
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.

Is quasiquotation in Core a good idea?
2 participants