Skip to content

Properly scope GADT equality evidence in the judgment #1942

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

Merged
merged 3 commits into from
Jun 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions plugins/hls-tactics-plugin/src/Wingman/CodeGen.hs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ destructMatches use_field_puns f scrut t jdg = do
-- #syn_scoped
method_hy = foldMap evidenceToHypothesis ev
args = conLikeInstOrigArgTys' con apps
modify $ evidenceToSubst ev
subst <- gets ts_unifier
ctx <- ask

let names_in_scope = hyNamesInScope hy
Expand All @@ -80,7 +78,7 @@ destructMatches use_field_puns f scrut t jdg = do
let hy' = patternHypothesis scrut con jdg
$ zip names'
$ coerce args
j = fmap (CType . substTyAddInScope subst . unCType)
j = withNewCoercions (evidenceToCoercions ev)
$ introduce ctx hy'
$ introduce ctx method_hy
$ withNewGoal g jdg
Expand Down
11 changes: 11 additions & 0 deletions plugins/hls-tactics-plugin/src/Wingman/Judgements.hs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import OccName
import SrcLoc
import Type
import Wingman.GHC (algebraicTyCon, normalizeType)
import Wingman.Judgements.Theta
import Wingman.Types


Expand Down Expand Up @@ -69,6 +70,15 @@ withNewGoal :: a -> Judgement' a -> Judgement' a
withNewGoal t = field @"_jGoal" .~ t


------------------------------------------------------------------------------
-- | Add some new type equalities to the local judgement.
withNewCoercions :: [(CType, CType)] -> Judgement -> Judgement
withNewCoercions ev j =
let subst = allEvidenceToSubst mempty $ coerce ev
in fmap (CType . substTyAddInScope subst . unCType) j
& field @"j_coercion" %~ unionTCvSubst subst


normalizeHypothesis :: Functor f => Context -> f CType -> f CType
normalizeHypothesis = fmap . coerce . normalizeType

Expand Down Expand Up @@ -414,6 +424,7 @@ mkFirstJudgement ctx hy top goal =
, _jWhitelistSplit = True
, _jIsTopHole = top
, _jGoal = CType goal
, j_coercion = emptyTCvSubst
}


Expand Down
12 changes: 9 additions & 3 deletions plugins/hls-tactics-plugin/src/Wingman/Judgements/Theta.hs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ module Wingman.Judgements.Theta
( Evidence
, getEvidenceAtHole
, mkEvidence
, evidenceToCoercions
, evidenceToSubst
, evidenceToHypothesis
, evidenceToThetaType
, allEvidenceToSubst
) where

import Class (classTyVars)
import Control.Applicative (empty)
import Control.Lens (preview)
import Data.Coerce (coerce)
import Data.Maybe (fromMaybe, mapMaybe, maybeToList)
import Data.Generics.Sum (_Ctor)
import Data.Set (Set)
Expand Down Expand Up @@ -109,14 +112,17 @@ allEvidenceToSubst skolems ((a, b) : evs) =
$ allEvidenceToSubst skolems
$ fmap (substPair subst) evs

------------------------------------------------------------------------------
-- | Given some 'Evidence', get a list of which types are now equal.
evidenceToCoercions :: [Evidence] -> [(CType, CType)]
evidenceToCoercions = coerce . mapMaybe (preview $ _Ctor @"EqualityOfTypes")

------------------------------------------------------------------------------
-- | Update our knowledge of which types are equal.
evidenceToSubst :: [Evidence] -> TacticState -> TacticState
evidenceToSubst evs ts =
updateSubst
(allEvidenceToSubst (ts_skolems ts)
$ mapMaybe (preview $ _Ctor @"EqualityOfTypes")
$ evs)
(allEvidenceToSubst (ts_skolems ts) . coerce $ evidenceToCoercions evs)
ts


Expand Down
13 changes: 12 additions & 1 deletion plugins/hls-tactics-plugin/src/Wingman/Machinery.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ substCTy :: TCvSubst -> CType -> CType
substCTy subst = coerce . substTy subst . coerce


getSubstForJudgement
:: MonadState TacticState m
=> Judgement
-> m TCvSubst
getSubstForJudgement j = do
-- NOTE(sandy): It's OK to use mempty here, because coercions _can_ give us
-- substitutions for skolems.
let coercions = j_coercion j
unifier <- gets ts_unifier
pure $ unionTCvSubst unifier coercions

------------------------------------------------------------------------------
-- | Produce a subgoal that must be solved before we can solve the original
-- goal.
Expand All @@ -48,7 +59,7 @@ newSubgoal
-> Rule
newSubgoal j = do
ctx <- ask
unifier <- gets ts_unifier
unifier <- getSubstForJudgement j
subgoal
$ normalizeJudgement ctx
$ substJdg unifier
Expand Down
8 changes: 6 additions & 2 deletions plugins/hls-tactics-plugin/src/Wingman/Tactics.hs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import Control.Monad (unless)
import Control.Monad.Except (throwError)
import Control.Monad.Extra (anyM)
import Control.Monad.Reader.Class (MonadReader (ask))
import Control.Monad.State.Strict (StateT(..), runStateT, gets)
import Control.Monad.State.Strict (StateT(..), runStateT)
import Data.Bool (bool)
import Data.Foldable
import Data.Functor ((<&>))
Expand Down Expand Up @@ -72,6 +72,10 @@ assume name = rule $ \jdg -> do
recursion :: TacticsM ()
-- TODO(sandy): This tactic doesn't fire for the @AutoThetaFix@ golden test,
-- presumably due to running afoul of 'requireConcreteHole'. Look into this!

-- TODO(sandy): There's a bug here! This should use the polymorphic defining
-- types, not the ones available via 'getCurrentDefinitions'. As it is, this
-- tactic doesn't support polymorphic recursion.
recursion = requireConcreteHole $ tracing "recursion" $ do
defs <- getCurrentDefinitions
attemptOn (const defs) $ \(name, ty) -> markRecursion $ do
Expand Down Expand Up @@ -349,7 +353,7 @@ destructAll = do
$ unHypothesis
$ jHypothesis jdg
for_ args $ \arg -> do
subst <- gets ts_unifier
subst <- getSubstForJudgement =<< goal
destruct $ fmap (coerce substTy subst) arg

--------------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions plugins/hls-tactics-plugin/src/Wingman/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,9 @@ data Judgement' a = Judgement
, _jWhitelistSplit :: !Bool
, _jIsTopHole :: !Bool
, _jGoal :: !a
, j_coercion :: TCvSubst
}
deriving stock (Eq, Generic, Functor, Show)
deriving stock (Generic, Functor, Show)

type Judgement = Judgement' CType

Expand Down Expand Up @@ -328,7 +329,6 @@ data TacticError
| TooPolymorphic
| NotInScope OccName
| TacticPanic String
deriving stock (Eq)

instance Show TacticError where
show (UndefinedHypothesis name) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ spec = do
metaTest 11 11 "MetaUseMethod"
metaTest 9 38 "MetaCataCollapse"
metaTest 7 16 "MetaCataCollapseUnary"
metaTest 21 32 "MetaCataAST"
metaTest 10 32 "MetaCataAST"
metaTest 6 46 "MetaPointwise"
metaTest 4 28 "MetaUseSymbol"
13 changes: 1 addition & 12 deletions plugins/hls-tactics-plugin/test/golden/MetaCataAST.expected.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,8 @@ data AST a where
Equal :: AST a -> AST a -> AST Bool

eval :: AST a -> a
-- NOTE(sandy): There is an unrelated bug that is shown off in this test
-- namely, that
--
-- @eval (IntLit n) = _@
--
-- but should be
--
-- @eval (IntLit n) = n@
--
-- https://github.com/haskell/haskell-language-server/issues/1937

eval (BoolLit b) = b
eval (IntLit n) = _
eval (IntLit n) = n
eval (If ast ast' ast_a)
= let
ast_c = eval ast
Expand Down
11 changes: 0 additions & 11 deletions plugins/hls-tactics-plugin/test/golden/MetaCataAST.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,5 @@ data AST a where
Equal :: AST a -> AST a -> AST Bool

eval :: AST a -> a
-- NOTE(sandy): There is an unrelated bug that is shown off in this test
-- namely, that
--
-- @eval (IntLit n) = _@
--
-- but should be
--
-- @eval (IntLit n) = n@
--
-- https://github.com/haskell/haskell-language-server/issues/1937

eval = [wingman| intros x, cata x; collapse |]