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

Specs of local definitions #2325

Open
facundominguez opened this issue Sep 3, 2024 · 7 comments
Open

Specs of local definitions #2325

facundominguez opened this issue Sep 3, 2024 · 7 comments

Comments

@facundominguez
Copy link
Collaborator

facundominguez commented Sep 3, 2024

Specs of local definitions have been problematic for some time now. In what follows I explain why this is brittle, and what we can do to improve it.

The problem

Ideally, local specs work as follows:

type Vec2D = Int -> Int -> Int

{-@ sum2D :: Vec2D -> Nat -> Nat -> Int @-}   
sum2D :: Vec2D -> Int -> Int -> Int
sum2D a n m = go n m
  where
    {-@ go :: ink:Nat -> joe:Nat -> Int / [ink, joe] @-}
    go 0 0        = a 0 0
    go i j
      | j == 0    =  a i 0 + go (i-1) m
      | otherwise =  a i j + go i (j-1)

In the above program, the go spec is linked to the go local definition.

However, sometimes GHC might desugar to the following core.

sum2D a n m =
 let go b1 b2 =
      let go2 0 0        = a 0 0
          go2 i j
            | j == 0    =  a i 0 + go2 (i-1) m
            | otherwise =  a i j + go2 i (j-1)
       in go2 b1 b2
  in go n m

The go spec is still linked to the go local definition. But note that go is no longer a recursive definition. go2 is a recursive definition, but it cannot be proved to terminate because the termination metrics are attached to go instead of go2.

This case used to pass in the testsuite because a-normalization would rearrange it to

sum2D a n m =
    {-@ go :: ink:Nat -> joe:Nat -> Int / [ink, joe] @-}
    let go2 0 0        = a 0 0
        go2 i j
          | j == 0    =  a i 0 + go2 (i-1) m
          | otherwise =  a i j + go2 i (j-1)
     in let go b1 b2 = go2 b1 b2
         in go n m

(I'm eliding the other effects of a-normalization here)
and, go and go2 Vars are mapped to the same go symbol in LH. This is because when GHC generates go2 it gives it the same string name as go but a different unique. Moreover, go2 is given the same source location as go, which is the main attribute that LH uses to decide what to attach a spec to.

Slight changes to the example or GHC are likely to upset this equilibrium, though, as I have encountered in practice already.

Earlier measures

Liquid Haskell does some transformations on the desugared expressions, in an attempt to prevent this sort of trouble. For this case, this inlineLoopBreaker transformation would apply if the output of desugaring were instead:

sum2D a n m =
 let go =
      let go2 0 0        = a 0 0
          go2 i j
            | j == 0    =  a i 0 + go2 (i-1) m
            | otherwise =  a i j + go2 i (j-1)
       in go2
  in go n m

Note that there is an eta reduction, and the parameters b1 and b2 are no longer present. inlineLoopBreaker would then produce almost the original program as written by the user.

sum2D a n m =
 let go 0 0       = a 0 0
     go i j
       | j == 0    = a i 0 + go2 (i-1) m
       | otherwise = a i j + go2 i (j-1)
  in go n m

While we could try to accommodate this case in the same transformation, I would prefer that we aimed for a simpler solution, or one that could survive to more tweaks to desugaring.

A simpler solution

Perhaps Liquid Haskell should flag an error when there is a spec to a local binding, and there are multiple such local bindings with the same name and source location. This would cause the above program to fail verification.

The user then could stop GHC from generating a go2 binding if go is given an explicit type signature. The following example would pass verification because there is only one go binding with the closest source location.

{-@ sum2D :: Vec2D -> Nat -> Nat -> Int @-}   
sum2D :: Vec2D -> Int -> Int -> Int
sum2D a n m = go n m
  where
    {-@ go :: ink:Nat -> joe:Nat -> Int / [ink, joe] @-}
    go :: Int -> Int -> Int
    go 0 0        = a 0 0
    go i j
      | j == 0    =  a i 0 + go (i-1) m
      | otherwise =  a i j + go i (j-1)

Is this enough of a remedy?

@facundominguez
Copy link
Collaborator Author

facundominguez commented Sep 3, 2024

Here's another proposal: We should always link the spec to a recursive binding if there is more than one closest binding at the same source location. If there are no recursive bindings, just pick any of the closest bindings. We could have only non-recursive bindings if we are trying to annotate a non-recursive value or function.

I don't expect to have to decide between multiple recursive bindings, or bindings that have different types. We will have to consider those cases when they happen.

To implement the change, the local bindings are collected here, and they are only used here.

I'm storing the work in progress for this strategy here:
https://github.com/ucsd-progsys/liquidhaskell/compare/fd/local-binds-specs

@facundominguez
Copy link
Collaborator Author

facundominguez commented Sep 6, 2024

Here's another example that doesn't work with the above strategy.

{-@ LIQUID "--ple" @-}                                                                                                                                                                                                                                                                                                                                                                      module Test where                                                                                                                                                                                                                                                                                                                                                                           {-@ reflect one @-}                                                                                                                                                                           one :: Int -> Int                                                                                                                                                                             
one 1 = 1                                                                                                                                                                                     
one _ = 0                                                                                                                                                                                     
                                                                                                                                                                                              
{-@ f :: {v:Int | v > 0} @-}                                                                                                                                                                  
f = go 1 2                                                                                                                                                                                    
  where                                                                                                                                                                                       
    {-@ go :: i:Int -> {x:Int | x >= 0 && one i >= 0} -> {v:Int | v > 0} / [x] @-}                                                                                                            
--  Uncomenting the haskell signature causes desugaring to produce only                                                                                                                       
--  one binding of go                                                                                                                                                                         
--  go :: Int -> Int -> Int                                                                                                                                                                   
    go _ 0 = 1 :: Int                                                                                                                                                                         
    go i n = go (i :: Int) (n - 1 :: Int)

Desugars to

one                                            
  = \ ds_d1Xe ->
      case ds_d1Xe of { I# ds_d1Xf ->
      case ds_d1Xf of {
        __DEFAULT -> I# 0#;
        1# -> I# 1#
      }
      }

f = join {
      go_aVf eta_B2 eta_B1
        = letrec {
            go_aVx
              = \ ds_d1X7 ds_d1X8 ->
                  case ds_d1X8 of { I# ds_d1X9 ->
                  case ds_d1X9 of {
                    __DEFAULT -> go_aVx ds_d1X7 (- $fNumInt ds_d1X8 (I# 1#));
                    0# -> I# 1#
                  }
                  }; } in
          go_aVx eta_B2 eta_B1 } in
    jump go_aVf (I# 1#) (I# 2#)

$trModule = Module (TrNameS "main"#) (TrNameS "Test"#)

The go spec is linked to the recursive go_aVx, but LH complains about the application go_aVx eta_B2 eta_B1.

Test.hs:17:5: error: 
    Liquid Type Mismatch                   
    .                                                                                          
    The inferred type                                                                          
      VV : {v : GHC.Types.Int | v == eta_B1                                                    
                                && $VV##266##k_ /= 0                                           
                                && $VV##266##k_ /= $eta_B2##k_                                 
                                && $VV##266##k_ > 0                                            
                                && $VV##266##k_ > $eta_B2##k_
                                && $VV##266##k_ >= 0
                                && $VV##266##k_ >= $eta_B2##k_}              
    .                          
    is not a subtype of the required type
      VV : {VV##425 : GHC.Types.Int | Test.one eta_B2 >= 0}
    .                              
    in the context                                                                             
      eta_B2 : GHC.Types.Int                                                                   
                                                                                               
      eta_B1 : {eta_B1 : GHC.Types.Int | $VV##266##k_ /= 0                                     
                                         && $VV##266##k_ /= $eta_B2##k_                        
                                         && $VV##266##k_ > 0                                   
                                         && $VV##266##k_ > $eta_B2##k_                                                                                                                        
                                         && $VV##266##k_ >= 0                                                                                                                                 
                                         && $VV##266##k_ >= $eta_B2##k_}                                                                                                                      
    Constraint id 7                                                                                                                                                                           
   |                                                                                                                                                                                          
17 |     go _ 0 = 1 :: Int                                                                                                                                                                    
   |     ^^

it cannot show that eta_B1 satisfies the precondition of the second parameter of go.

Normally, LH would be able to infer a suitable refinement type for eta_B1, but PLE is at play here, and apparently prevents the inference from happening.

@facundominguez
Copy link
Collaborator Author

facundominguez commented Sep 16, 2024

Another complicated case arises when compiling tests/benchmarks/bytestring-0.9.2.1/Data/ByteString/Fusion/T.hs without -fbreak-point.

loopU has a recursive binding called trans, nested inside another binding go.

Without break points, verification fails with

benchmarks/bytestring-0.9.2.1/Data/ByteString/Fusion/T.hs:262:65: error:                                                                                                    20:34:33 [34/1835]
    Liquid Type Mismatch                 
    .                                                                                          
    The inferred type
      VV : GHC.Types.Int
    .                                                                                          
    is not a subtype of the required type
      VV : {VV##31082 : GHC.Types.Int | VV##31082 <= i##a3cB - ?a}
    .                                                                                          
    in the context                                                                             
      z##a3cz : (GHC.Internal.ForeignPtr.ForeignPtr GHC.Internal.Word.Word8)
                                                                                               
      i##a3cB : {i##a3cB : GHC.Types.Int | i##a3cB >= 0
                                           && s##a3cA + i##a3cB <= fplen z##a3cz}
                     
      ?a : GHC.Types.Int                                                                       
       
      s##a3cA : {s##a3cA : GHC.Types.Int | s##a3cA >= 0
                                           && s##a3cA <= fplen z##a3cz}
    Constraint id 1018
    |
262 | loopU f start (PS z s i) = unsafePerformIO $ withForeignPtr z $ \a -> do
    |                                                                 ^^^^^^^^...

benchmarks/bytestring-0.9.2.1/Data/ByteString/Fusion/T.hs:276:40: error:
    Liquid Type Mismatch
    .
    The inferred type
      VV : {v : GHC.Types.Int | v == b##a3cH
                                && $VV##3579##k_ >= $eta_B2##k_}
    .
    is not a subtype of the required type
      VV : {VV##31643 : GHC.Types.Int | 0 <= VV##31643}
    .
    in the context
      b##a3cH : {b##a3cH : GHC.Types.Int | $VV##3579##k_ >= $eta_B2##k_}
    Constraint id 1124
    |
276 |                     x <- peekByteOff p a_off
    |                                        ^^^^^


benchmarks/bytestring-0.9.2.1/Data/ByteString/Fusion/T.hs:280:55: error:
    Liquid Type Mismatch
    .
    The inferred type
      VV : {v : GHC.Types.Int | v == c##a3cI
                                && $VV##3581##k_ >= $eta_B1##k_}
    .
    is not a subtype of the required type
      VV : {VV##31621 : GHC.Types.Int | 0 <= VV##31621}
    .
    in the context
      c##a3cI : {c##a3cI : GHC.Types.Int | $VV##3581##k_ >= $eta_B1##k_}
    Constraint id 1115
    |
280 |                         JustS e  -> do pokeByteOff ma ma_off e
    |                                                       ^^^^^^

benchmarks/bytestring-0.9.2.1/Data/ByteString/Fusion/T.hs:282:27: error:
    Liquid Type Mismatch
    .
    The inferred type
      VV : {v : GHC.Types.Int | v == a##a3cG - (1 : int)}
    .
    is not a subtype of the required type
      VV : {VV##31582 : GHC.Types.Int | VV##31582 >= 0}
    .
    in the context
      a##a3cG : {a##a3cG : GHC.Types.Int | $VV##3577##k_ <= $i##a3cB##k_
                                           && $VV##3577##k_ <= plen $ma##a3cE##k_
                                           && $VV##3577##k_ <= plen $p##a3cD##k_
                                           && $VV##3577##k_ <= $i##a3cB##k_ + 1
                                           && $VV##3577##k_ <= $i##a3cB##k_ - $lq_anf$##7205759403792813058##d53Q##k_
                                           && $VV##3577##k_ <= $i##a3cB##k_ - $lq_anf$##7205759403792813059##d53R##k_}
    Constraint id 1106
    |
282 |                     trans (d-1) (a_off+1) ma_off' acc'
    |                           ^^^^^

Similar to the example in the previous comment, these errors are avoided by giving an explicit type signature to go, which has an effect on desugaring, which has an effect in verification that I don't understand yet.

@simonpj
Copy link
Collaborator

simonpj commented Sep 19, 2024

I think there is a simpler solution here: give the local function a Haskell type signature.

When you do that, GHC will not generate an auxiliary local binding. Here's your example:

type Vec2D = Int -> Int -> Int

{-@ sum2D :: Vec2D -> Nat -> Nat -> Int @-}
sum2D :: Vec2D -> Int -> Int -> Int
sum2D a n m = go n m
  where
    {-@ go :: ink:Nat -> joe:Nat -> Int / [ink, joe] @-}
    go :: Int -> Int -> Int     -- Note the signature!
    go 0 0        = a 0 0
    go i j
      | j == 0    =  a i 0 + go (i-1) m
      | otherwise =  a i j + go i (j-1)

We get this from -ddump-ds-preopt:

sum2D :: Vec2D -> Int -> Int -> Int
[LclIdX]
sum2D
  = \ (a_awu :: Vec2D) (n_awv :: Int) (m_aww :: Int) ->
      letrec {
        go_awx :: Int -> Int -> Int
        [LclId]
        go_awx
          = \ (ds_dOO :: Int) (ds_dOP :: Int) ->
              let {
                fail_dOV :: (# #) -> Int
                [LclId]
                fail_dOV
                  = \ (ds_dOW [OS=OneShot] :: (# #)) ->
                      case GHC.Internal.Control.Exception.Base.patError
                             @GHC.Types.LiftedRep @() "T24386.hs:(11,5)-(14,39)|function go"#
                      of {} } in
              let {
                i_awy :: Int
                [LclId]
                i_awy = ds_dOO } in
              let {
                fail_dOZ :: (# #) -> Int
                [LclId]
                fail_dOZ
                  = \ (ds_dP0 [OS=OneShot] :: (# #)) ->
                      let {
                        j_awz :: Int
                        [LclId]
                        j_awz = ds_dOP } in
                      let {
                        fail_dOX :: (# #) -> Int
                        [LclId]
                        fail_dOX
                          = \ (ds_dOY [OS=OneShot] :: (# #)) ->
                              + @Int
                                $dNum_aO0
                                (a_awu i_awy j_awz)
                                (go_awx i_awy (- @Int $dNum_aO8 j_awz (GHC.Types.I# 1#))) } in
                      case == @Int $dEq_aNE j_awz (GHC.Types.I# 0#) of wild_00 {
                        False -> fail_dOX GHC.Types.(##);
                        True ->
                          + @Int
                            $dNum_aNL
                            (a_awu i_awy (GHC.Types.I# 0#))
                            (go_awx (- @Int $dNum_aNR i_awy (GHC.Types.I# 1#)) m_aww)
                      } } in
              case ds_dOO of wild_00 { GHC.Types.I# ds_dOT ->
              case ds_dOT of ds_dOT {
                __DEFAULT -> fail_dOZ GHC.Types.(##);
                0# ->
                  case ds_dOP of wild_00 { GHC.Types.I# ds_dOU ->
                  case ds_dOU of ds_dOU {
                    __DEFAULT -> fail_dOZ GHC.Types.(##);
                    0# -> a_awu (GHC.Types.I# 0#) (GHC.Types.I# 0#)
                  }
                  }
              }
              }; } in
      go_awx n_awv m_aww
end Rec }

Now your job is much easier I think.

This works for mutual recursion. Functions that have a complete type signature always go this route.

@facundominguez
Copy link
Collaborator Author

facundominguez commented Sep 19, 2024

Thanks @simonpj for having a look. I guess at this point it is a matter of quality of the user experience.

Should we have LH complain when a spec is given for a local binding that has no type signature?

Adding the signature is always going to be some hassle, and it may be worse if -XScopedTypeVariables needs to be enabled and type variables need to be quantified.

Would it be possible to talk the desugarer into avoiding auxiliary bindings regardless of the presence of type signatures? Perhaps this would be ideal from the user perspective.

@simonpj
Copy link
Collaborator

simonpj commented Sep 19, 2024

Surely the Haskell type sig is just a subset of the LH spec? Easy!

Would it be possible to talk the desugarer into avoiding auxiliary bindings regardless of the presence of type signatures? Perhaps this would be ideal from the user perspective.

I'm afraid it would not. It's to do with typechecking mutual recursion. The recursive loop is monomorphic, even though the external interface is polymorphic.

@facundominguez
Copy link
Collaborator Author

facundominguez commented Sep 20, 2024

Surely the Haskell type sig is just a subset of the LH spec? Easy!

Yes ... when the spec is complete. Sometimes the user could leave it to GHC. e.g.

{-@ go :: _ -> _ -> _ @-}

Is the guarantee to eliminate auxiliary bindings via explicit type signatures something the desugarer could have tests for?

I'm afraid it would not.

That's good to know. We need to collect more examples, though. With the last tuning of Core transformations, the need for explicit type signatures is not surfacing in tests. But there is no design at work to stop it from coming back later.

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

No branches or pull requests

2 participants