ucsd-progsys / liquid-fixpoint

Horn Clause Constraint Solving for Liquid Types
BSD 3-Clause "New" or "Revised" License
141 stars 61 forks source link

Extensional reasoning on Values #705

Closed AlecsFerra closed 1 month ago

AlecsFerra commented 2 months ago

The extensionality flag fails when a function used as argument of a data constructor, take this as an example:

fixpoint "--rewrite"
fixpoint "--allowho"

constant f : (func(0 , [int; int; int]))
define f (x : int, y: int) : int = {(x + y)}

constant g : (func(0 , [int; int; int]))
define g (a : int, b: int) : int = {(b + a)}

data Ty 0 = [
    | Cons {mkCons : func(0 , [int; int; int])}
]

constant Cons : (func(0 , [func(0 , [int; int; int]); Ty]))

expand [1 : True; 2 : True]

constraint:
  env []
  lhs {VV1 : Tuple | true }
  rhs {VV2 : Tuple | (Cons f = Cons g) }
  id 2 tag []

it cannot prove that f and g are the same function since it cannot add the ext$... variable on the functions (and then cause their unfolding by PLE) since they would make the program ill-typed, the solution is eta-expanding the definitions of f and g to (\x:Int -> \y:Int -> f x y) (\x:Int -> \y:Int -> g x y) to let PLE then unfold the definitions in the body of the lambda and prove the equality.

DO NOT MERGE: This feature is exposing another LF bug in some tests in the liquidhaskell repo related to lambdas in the refinements.

AlecsFerra commented 2 months ago

The issue on the liquid haskell main repo is related to programs like this:

{-@ LIQUID "--reflection" @-}
{-@ LIQUID "--ple"        @-}

module Example where

{-@ reflect idu @-}
{-@ idu :: Int -> Int @-}
idu :: Int -> Int
idu x = x

{-@ proof :: {(\x:Int -> x) = idu } @-}
proof = ()

As it throws crash: SMTLIB2 respSat = Error "line 3 column 18650: unknown constant lam_arg$35$$35$1" whenever a lambda is used this way in the refinements

This error is already present is not caused by this patch

nikivazou commented 2 months ago

Yes, right now there is no real support for lambdas in LH :)

AlecsFerra commented 2 months ago

Uhm ok then I'm stuck

AlecsFerra commented 2 months ago

Right now It's verifying the example but it's also making some tests in the LH repo fail because it's unfolding some types and inserting lambdas

AlecsFerra commented 2 months ago

I think I fixed the bug regarding lambdas, I'll run the tests on the main repo to double check

AlecsFerra commented 2 months ago

The tests on the liquidhaskell are passing!

But I still have to fix a minor bug for my tests

AlecsFerra commented 2 months ago

Can confirm that it's passing all the test on the main LH repo, now I'm running the benchmarks

AlecsFerra commented 2 months ago

Benchmarks: filtered top bot

On average we are not extremely slower than then original PLE

AlecsFerra commented 1 month ago

Still in todo:

AlecsFerra commented 1 month ago

Ops forgot about the tests on liquid-fixpoint

AlecsFerra commented 1 month ago

filtered top bot

AlecsFerra commented 1 month ago

I can open a PR on LH when we are done here https://github.com/AlecsFerra/liquidhaskell/commits/develop/ I've also added two tests to showcase the feature:

AlecsFerra commented 1 month ago

Btw I think that now we can allow lambdas in reflected functions definitions, at least for the reader monad example it works

AlecsFerra commented 1 month ago

I will squash the commits at the end

AlecsFerra commented 1 month ago

Following some discussion in private I decided to remove the code relating to make PLE aware of dependent pattern matching / unification

The code of the reader monad still passes, the SKI calculus instead will require some extra lemmas

facundominguez commented 1 month ago

The only open discussion that I could catch is about making a comment more instructive. Update: oh, I asked about a couple other things in comments later.

Regarding performance, if the verification times worsen when enabling extensionality and --etabeta I'd say it is fine to merge. If verification worsens even when not enabling the flags, then I'd prefer to understand why before merging.

AlecsFerra commented 1 month ago

filtered bot top

AlecsFerra commented 1 month ago

filtered bot top

AlecsFerra commented 1 month ago

Looks way better I guess the slowdowns were actually caused by the elaboration

facundominguez commented 1 month ago

Also, note the comment edits that I just contributed. Feel free to adjust them if you see anything amiss.

AlecsFerra commented 1 month ago

Ops

AlecsFerra commented 1 month ago

Ok now It's ok to merge