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

Fix unsafe data constructor refinements #2470

Merged
merged 9 commits into from
Jan 14, 2025
17 changes: 17 additions & 0 deletions docs/mkDocs/docs/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,23 @@ worse = bad (Very bad)
Note that all positive occurrences are permited, unlike Coq that only allows the strictly positive ones
(see: https://vilhelms.github.io/posts/why-must-inductive-types-be-strictly-positive/)

## Refined construcotrs
AlecsFerra marked this conversation as resolved.
Show resolved Hide resolved

**Options:** `allow-unsafe-constructors`
By **default** only safe constructor refinements are allowed.
AlecsFerra marked this conversation as resolved.
Show resolved Hide resolved

Allows the user to refine the return type of a data constructor with arbitrary refinements, also unsafe ones.

```haskell
data F where
{-@ MkF :: { _:F | false } @-}
MkF :: F

{-@ bad :: { false } @-}
bad :: F
bad = () ? MkF
```

## Total Haskell

**Options:** `total-Haskell`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ _ *** _ = ()
data QED = Admit | QED

{-@ measure isAdmit :: QED -> Bool @-}
{-@ Admit :: {v:QED | isAdmit v } @-}
isAdmit :: QED -> Bool
isAdmit Admit = True
isAdmit QED = False


-------------------------------------------------------------------------------
Expand Down
36 changes: 36 additions & 0 deletions liquidhaskell-boot/src/Language/Haskell/Liquid/Bare/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ checkTargetSpec specs src env cbs tsp
<> checkSizeFun emb env (gsTconsP (gsName tsp))
<> checkPlugged (catMaybes [ fmap (F.dropSym 2 $ GM.simplesymbol x,) (getMethodType t) | (x, t) <- gsMethods (gsSig tsp) ])
<> checkRewrites tsp
<> if allowUnsafeConstructors $ getConfig tsp
then mempty
else checkConstructorRefinement (gsTySigs $ gsSig tsp)

_rClasses = concatMap Ms.classes specs
_rInsts = concatMap Ms.rinstance specs
Expand All @@ -193,6 +196,39 @@ checkTargetSpec specs src env cbs tsp



-- | Tests that the returned refinement type of data constructors has predicate @True@ or @prop v == e@.
--
-- > data T = T Int
-- > {-@ T :: x:Int -> { v:T | v = T x } @-} -- Should be rejected
-- > {-@ T :: x:Int -> { v:T | True } @-} -- Should be fine
-- > {-@ T :: x:Int -> { v:T | prop v = True } @-} -- Should be fine
--
checkConstructorRefinement :: [(Var, LocSpecType)] -> Diagnostics
AlecsFerra marked this conversation as resolved.
Show resolved Hide resolved
checkConstructorRefinement = mconcat . map checkOne
where
checkOne (s, ty) | isCtorName s
, not $ validRef $ getRetTyRef $ val ty
= mkDiagnostics mempty [ ErrCtorRefinement (GM.sourcePosSrcSpan $ loc ty) (pprint s) ]
checkOne _ = mempty

getRetTyRef (RFun _ _ _ t _) = getRetTyRef t
getRetTyRef (RAllT _ t _) = getRetTyRef t
getRetTyRef t = ur_reft $ rt_reft t

-- True refinement
validRef (F.Reft (_, F.PTrue))
= True
-- Prop foo from ProofCombinators
validRef (F.Reft (v, F.PAtom F.Eq (F.EApp (F.EVar n) (F.EVar v')) _))
| n == "Language.Haskell.Liquid.ProofCombinators.prop"
, v == v'
= True
Comment on lines +222 to +225
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be so strict? Maybe it is fine to allow both prop v = foo and foo = prop v.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No is fine also in the other direction, I just copied the way that is done in ProofCombinators

validRef _ = False

isCtorName x = case idDetails x of
DataConWorkId _ -> True
DataConWrapId _ -> True
_ -> False


checkPlugged :: PPrint v => [(v, LocSpecType)] -> Diagnostics
Expand Down
11 changes: 11 additions & 0 deletions liquidhaskell-boot/src/Language/Haskell/Liquid/Types/Errors.hs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ data TError t =
, dc :: !Doc
}

| ErrCtorRefinement { pos :: !SrcSpan
, ctorName :: !Doc
} -- ^ The refinement of a data constructor doesn't admit
-- a refinement on the return type that
-- isn't deemd safe

| ErrOther { pos :: SrcSpan
, msg :: !Doc
Expand Down Expand Up @@ -1061,6 +1066,12 @@ ppError' _ dCtx (ErrPosTyCon _ tc dc)
, nest 2 "https://ucsd-progsys.github.io/liquidhaskell/options/#positivity-check"
]

ppError' _ dCtx (ErrCtorRefinement _ ctorName)
= text "Refinement of the data constructor" <+> ctorName <+> "doesn't admit an arbitrary refinements on the return type"
$+$ dCtx
$+$ nest 4 (text "Were you trying to use `Prop` from `Language.Haskell.Liquid.ProofCombinators`?")
Comment on lines +1070 to +1072
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm realizing the error message can be more precise to tell users exactly which predicates are allowed: "Please use either of True or prop v = ..."

Copy link
Collaborator

Choose a reason for hiding this comment

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

" ... or flag --allow-unsafe-constructors"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree on mentioning the flag but idk about exposing the user to prop when in practice you always want to use Prop

$+$ nest 4 (text "You can disable this error by enabling the flag `--allow-unsafe-constructors`")

ppError' _ dCtx (ErrParseAnn _ msg)
= text "Malformed annotation"
$+$ dCtx
Expand Down
5 changes: 5 additions & 0 deletions liquidhaskell-boot/src/Language/Haskell/Liquid/UX/CmdLine.hs
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,10 @@ config = cmdArgsMode $ Config {
= def &= help "Dump pre-normalized core (before a-normalization)"
&= name "dump-pre-normalized-core"
&= explicit
, allowUnsafeConstructors
= def &= help "Allow refining constructors with unsafe refinements"
&= name "allow-unsafe-constructors"
&= explicit
} &= program "liquid"
&= help "Refinement Types for Haskell"
&= summary copyright
Expand Down Expand Up @@ -748,6 +752,7 @@ defConfig = Config
, excludeAutomaticAssumptionsFor = []
, dumpOpaqueReflections = False
, dumpPreNormalizedCore = False
, allowUnsafeConstructors = False
}

-- | Write the annotations (i.e. the files in the \".liquid\" hidden folder) and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ data Config = Config
, excludeAutomaticAssumptionsFor :: [String]
, dumpOpaqueReflections :: Bool -- Dumps all opaque reflections to the stdout
, dumpPreNormalizedCore :: Bool -- Dumps the prenormalized core (before a-normalization)
, allowUnsafeConstructors :: Bool -- ^ Allow refining constructors with unsafe refinements
} deriving (Generic, Data, Typeable, Show, Eq)

allowPLE :: Config -> Bool
Expand Down
4 changes: 3 additions & 1 deletion tests/benchmarks/cse230/src/Week10/ProofCombinators.hs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ _ *** _ = ()
data QED = Admit | QED

{-@ measure isAdmit :: QED -> Bool @-}
{-@ Admit :: {v:QED | isAdmit v } @-}
isAdmit :: QED -> Bool
isAdmit Admit = True
isAdmit QED = False


-------------------------------------------------------------------------------
Expand Down
17 changes: 17 additions & 0 deletions tests/neg/RefCtor.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{-# LANGUAGE GADTs #-}

-- | Tests that the returned refinement type of data constructors
-- is not allowed to be other than @True@.
module RefCtor where
AlecsFerra marked this conversation as resolved.
Show resolved Hide resolved

import Language.Haskell.Liquid.ProofCombinators

{-@ type K X Y = { _:F | X = Y } @-}

data F where
{-@ LIQUID "--expect-error-containing=Refinement of the data constructor RefCtor.MkF doesn't admit an arbitrary refinements on the return type" @-}
{-@ MkF :: x:Int -> y:Int -> K x y @-}
MkF :: Int -> Int -> F

{-@ falseProof :: { false } @-}
falseProof = () ? MkF 0 2
15 changes: 15 additions & 0 deletions tests/pos/AllowUnsafeCtor.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{-# LANGUAGE GADTs #-}

module AllowUnsafeCtor where

import Language.Haskell.Liquid.ProofCombinators

{-@ type K X Y = { _:F | X = Y } @-}

{-@ LIQUID "--allow-unsafe-constructors" @-}
data F where
{-@ MkF :: x:Int -> y:Int -> K x y @-}
MkF :: Int -> Int -> F

{-@ falseProof :: { false } @-}
falseProof = () ? MkF 0 2
2 changes: 0 additions & 2 deletions tests/pos/CountMonad.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ module CountMonad () where

{-@ measure count :: Count a -> Int @-}

{-@ Count :: x:a -> {v:Count a | v == Count x } @-}
data Count a = Count a

instance Functor Count where
Expand Down Expand Up @@ -36,7 +35,6 @@ assertCount _ x = x
getCount :: Count a -> Int
getCount _ = 0

{-@ makeCount :: x:a -> {v:Count a | v == Count x} @-}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this not true anymore? I would expect it to remain valid.

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 is still true bout you need --exact-data-cons or some other flag that triggers it

makeCount = Count

{-@ assume incr :: a -> {v:Count a | count v == 1 } @-}
Expand Down
1 change: 1 addition & 0 deletions tests/tests.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,7 @@ executable unit-neg
, Record0
, RecQSort
, RecSelector
, RefCtor
, RefinedProp
, Revshape
, ReWrite2
Expand Down
Loading