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

Incorrect reduction of pattern matching #346

Open
ziman opened this issue May 2, 2020 · 4 comments
Open

Incorrect reduction of pattern matching #346

ziman opened this issue May 2, 2020 · 4 comments

Comments

@ziman
Copy link
Collaborator

ziman commented May 2, 2020

Steps to Reproduce

import Data.Vect

%default total

infix 3 .<=.
(.<=.) : Int -> Int -> Bool
(.<=.) p q = case (p, q) of
  (0, _) => True
  _ => False

countGreater : (thr : Int) -> (ctx : Vect n Int) -> Nat
countGreater thr [] = Z
countGreater thr (x :: xs) with (x .<=. thr)
  countGreater thr (x :: xs) | True = countGreater thr xs
  countGreater thr (x :: xs) | False = S $ countGreater thr xs

-- map a variable from the old context to the erased context
-- where we erase all bindings less than or equal to the threshold
eraseVar : (thr : Int) -> (ctx : Vect n Int) -> Fin n -> Maybe (Fin (countGreater thr ctx))
eraseVar thr (x :: xs) j with (x .<=. thr)
  eraseVar thr (x :: xs) FZ | True = Nothing
  eraseVar thr (x :: xs) FZ | False = Just FZ
  eraseVar thr (x :: xs) (FS i) | True = FS <$> eraseVar thr xs i  -- WRONG
  eraseVar thr (x :: xs) (FS i) | False = FS <$> eraseVar thr xs i

boom : Maybe (Fin 1)
boom = eraseVar 0 [0, 1] (FS FZ)

{-

Main> :t boom
Main.boom : Maybe (Fin 1)
Main> boom
Just (FS FZ)

-}

Expected Behavior

The function eraseVar should fail to check on the RHS of its second-to-last clause.

The correct RHS, eraseVar thr xs i, should check.

Observed Behavior

The program checks and Idris claims that Just (FS FZ) : Maybe (Fin 1).

The correct RHS, eraseVar thr xs i, does not check.

Other observations

Somehow the definition of .<=. is crucial to this problem. If I use just <= from Prelude in the with expressions, everything works as expected.

@ziman
Copy link
Collaborator Author

ziman commented May 2, 2020

The following definition makes the program work as expected again.

(.<=.) : Int -> Int -> Bool
0 .<=. _ = True
_ .<=. _ = False

@ziman
Copy link
Collaborator Author

ziman commented May 2, 2020

I can manually lift the case expression.

foo : (Int, Int) -> Bool
foo (0, _) = True
foo _ = False

infix 3 .<=.
(.<=.) : Int -> Int -> Bool
(.<=.) p q = foo (p, q)

Then :di foo shows something interesting:

Compile time tree:
case {arg:0}[0] : [__] of
  { Builtin.MkPair {e:0} {e:1} {e:2} {e:3}
    => case {e:2}[2] : [__] of
        { 0 => Prelude.True
        | _ => Prelude.False
        }
  | _ => Prelude.False
  }

The generated case tree has a default branch, even though it's not necessary.

Indeed, the default branch does seem to get in the way because if I change the second clause of foo to be foo (_, _) = False, the bug is fixed.

I wonder if the evaluator somehow mistakenly concludes Mismatch on the MkPair constructor instead of getting stuck, and proceeds to the default case.

@ziman
Copy link
Collaborator Author

ziman commented May 2, 2020

    -- Default case matches against any *concrete* value
    tryAlt env loc opts fc stk val (DefaultCase sc) def
         = if concrete val
              then evalTree env loc opts fc stk sc def
              else def
      where
        concrete : NF free -> Bool
        concrete (NDCon _ _ _ _ _) = True
        concrete (NTCon _ _ _ _ _) = True
        concrete (NPrimVal _ _) = True
        concrete (NBind _ _ _ _) = True
        concrete (NType _) = True
        concrete _ = False

I'll try marking only Lam and Pi as concrete.

@ziman ziman changed the title Ill-typed with clauses Incorrect reduction of pattern matching May 2, 2020
@ziman
Copy link
Collaborator Author

ziman commented May 2, 2020

Changing the definition of concrete to the following does not seem to have any effect.

concrete : NF free -> Bool
concrete (NDCon _ _ _ _ _) = True
concrete (NTCon _ _ _ _ _) = True
concrete (NPrimVal _ _) = True
concrete (NBind _ _ (Lam _ _ _) _) = True
concrete (NBind _ _ (Pi _ _ _) _) = True
concrete (NType _) = True
concrete _ = False

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

1 participant