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

Deriving Data turns UNSAFE code to be SAFE #2432

Open
kleinreact opened this issue Nov 9, 2024 · 6 comments
Open

Deriving Data turns UNSAFE code to be SAFE #2432

kleinreact opened this issue Nov 9, 2024 · 6 comments

Comments

@kleinreact
Copy link

kleinreact commented Nov 9, 2024

The following example is reported to be liquid safe although it isn't. The problem seems to be introduced by the deriving (Data) clause, as the correct liquid type mismatch gets reported when leaving it out.

Tested with 8c550df and GHC 9.10.1.

{-@ LIQUID "" @-}
{-# LANGUAGE DeriveDataTypeable #-}
{-# OPTIONS_GHC -fplugin=LiquidHaskell #-}

module T2432 where

import Data.Data (Data)

{-@ data T = A | B  @-}
data T = A | B
  deriving (Data)

{-@ type A = { v : T | v == A } @-}

{-@ toInt :: v : A -> { i : Nat | i == 2 } @-}
toInt :: T -> Int
toInt A = 0
toInt _ = error ""

LH reports a cryptic warning at least

WARNING: Found false in Test.hs:1:1
WARNING: Found false in Test.hs:7:14-17
@kleinreact kleinreact changed the title Deriving Data tuns UNSAFE code to be SAFE Deriving Data turns UNSAFE code to be SAFE Nov 9, 2024
@AlecsFerra
Copy link
Contributor

My guess is that the code generated by deriving Data is introducing false in the scope

@AlecsFerra
Copy link
Contributor

I've minimized the example

module Unsound where

{-@ LIQUID "--save" @-}

import Data.Data

data T = A | B
   deriving Data
   
{-@ toInt :: T -> { i:Nat | false } @-}
toInt :: T -> Int
toInt A = -1
toInt B = undefined

Dumped the generated instance

==================== Derived instances ====================
Derived class instances:
  instance GHC.Internal.Data.Data.Data Unsound.T where
    GHC.Internal.Data.Data.gfoldl k_a18z z_a18A Unsound.A
      = z_a18A (\ -> Unsound.A)
    GHC.Internal.Data.Data.gfoldl k_a18B z_a18C Unsound.B
      = z_a18C (\ -> Unsound.B)
    GHC.Internal.Data.Data.gunfold k_a18D z_a18E c_a18F
      = case GHC.Internal.Data.Data.constrIndex c_a18F of
          GHC.Types.I# 1# -> z_a18E (\ -> Unsound.A)
          _ -> z_a18E (\ -> Unsound.B)
    GHC.Internal.Data.Data.toConstr Unsound.A = $cA_a18t
    GHC.Internal.Data.Data.toConstr Unsound.B = $cB_a18u
    GHC.Internal.Data.Data.dataTypeOf _ = $tT_a18s
  
  $tT_a18s :: GHC.Internal.Data.Data.DataType
  $cA_a18t :: GHC.Internal.Data.Data.Constr
  $cB_a18u :: GHC.Internal.Data.Data.Constr
  $tT_a18s
    = GHC.Internal.Data.Data.mkDataType
        "Unsound.T" [$cA_a18t, $cB_a18u]
  $cA_a18t
    = GHC.Internal.Data.Data.mkConstrTag
        $tT_a18s "A" 1 [] GHC.Internal.Data.Data.Prefix
  $cB_a18u
    = GHC.Internal.Data.Data.mkConstrTag
        $tT_a18s "B" 2 [] GHC.Internal.Data.Data.Prefix

Derived type family instances:



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.dataCast1 = GHC.Internal.Data.Data.$dmdataCast1
                                       @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.dataCast2 = GHC.Internal.Data.Data.$dmdataCast2
                                       @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapT = GHC.Internal.Data.Data.$dmgmapT
                                   @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapQl = GHC.Internal.Data.Data.$dmgmapQl
                                    @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapQr = GHC.Internal.Data.Data.$dmgmapQr
                                    @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapQ = GHC.Internal.Data.Data.$dmgmapQ
                                   @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapQi = GHC.Internal.Data.Data.$dmgmapQi
                                    @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapM = GHC.Internal.Data.Data.$dmgmapM
                                   @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapMp = GHC.Internal.Data.Data.$dmgmapMp
                                    @(Unsound.T)



==================== Filling in method body ====================
GHC.Internal.Data.Data.Data [Unsound.T]
  GHC.Internal.Data.Data.gmapMo = GHC.Internal.Data.Data.$dmgmapMo
                                    @(Unsound.T)

But when I manually insert the instance

module Unsound where

{-@ LIQUID "--save" @-}

import Data.Data

data T = A | B
    
instance Data T where
    gunfold k z c = case constrIndex c of
        1 -> z A
        _ -> z B

    gfoldl k z A = z A
    gfoldl k z B = z B

    toConstr A = cA_a18t
    toConstr B = cB_a18u

    dataTypeOf _ = tT_a18s

{-@ lazy tT_a18s @-}
{-@ lazy cA_a18t @-}
{-@ lazy cB_a18u @-}
tT_a18s :: DataType
cA_a18t :: Constr
cB_a18u :: Constr
tT_a18s = mkDataType "Unsound.T" [cA_a18t, cB_a18u]
cA_a18t = mkConstrTag tT_a18s "A" 1 [] Prefix
cB_a18u = mkConstrTag tT_a18s "B" 2 [] Prefix

{-@ toInt :: T -> { i:Nat | false } @-}
toInt :: T -> Int
toInt A = -1
toInt B = undefined

It fails the verification so it's some other weird interaction

@AlecsFerra
Copy link
Contributor

AlecsFerra commented Nov 13, 2024

Ok figured out the issue, the code generated by deriving introduces a k-var that is solved resolved to false

bind 180 fix$36$$36$tT_a1Ao : {VV##1611 : GHC.Internal.Data.Data.DataType | [$k_##1612]}
bind 181 fix$36$$36$cA_a1Ap : {VV##1613 : GHC.Internal.Data.Data.Constr | [$k_##1614]}
bind 182 fix$36$$36$cB_a1Aq : {VV##1615 : GHC.Internal.Data.Data.Constr | [$k_##1616]}

Instead when the code is manually inserted they don't get a k-var

bind 181 Unsound.tT_a18s : {VV##1612 : GHC.Internal.Data.Data.DataType | []}
bind 182 Unsound.cA_a18t : {VV##1613 : GHC.Internal.Data.Data.Constr | []}
bind 183 Unsound.cB_a18u : {VV##1614 : GHC.Internal.Data.Data.Constr | []}

I don't get why LH is putting there a k-var only when deriving is used, they supposedly have the same core representation

@AlecsFerra
Copy link
Contributor

CC @facundominguez

@facundominguez
Copy link
Collaborator

facundominguez commented Nov 13, 2024

👋 I see two issues here:

  1. Enable verification of derived instances with a flag. Perhaps an instance can be recognized as derived by looking at the source span of the definitions, and checking if they have been written in the source file.

  2. Investigate the mismatch in what LH does with derived code vs hand written code. I have no insights here at the moment.

@AlecsFerra
Copy link
Contributor

I think is has something to do with the definitions of Unsound.tT_a18s, Unsound.cA_a18t and Unsound.cB_a18u being non terminating as they are mutually recursive and clearly non terminating, I think that the kvars are set to false when performing termination checking

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

3 participants