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

Non-terminating function over tuples passed with --termination-depth=2 #6059

Closed
nad opened this issue Aug 30, 2022 · 5 comments · Fixed by #6069
Closed

Non-terminating function over tuples passed with --termination-depth=2 #6059

nad opened this issue Aug 30, 2022 · 5 comments · Fixed by #6069
Assignees
Labels
false Proof of the empty type which checks without known-unsafe flags (e.g. without --type-in-type) termination Issues relating to the termination checker type: bug Issues and pull requests about actual bugs
Milestone

Comments

@nad
Copy link
Contributor

nad commented Aug 30, 2022

In the following code there is special treatment of the cases "empty, singleton" and "singleton, empty", but in other cases where the lists have different lengths the extra elements are simply discarded:

compareConArgs :: Args -> [NamedArg DeBruijnPattern] -> TerM Order
compareConArgs ts ps = do
cutoff <- terGetCutOff
let ?cutoff = cutoff
-- we may assume |ps| >= |ts|, otherwise c ps would be of functional type
-- which is impossible
case (length ts, length ps) of
(0,0) -> return Order.le -- c <= c
(0,1) -> return Order.unknown -- c not<= c x
(1,0) -> __IMPOSSIBLE__
(1,1) -> compareTerm' (unArg (head ts)) (notMasked $ namedArg $ head ps)
(_,_) -> foldl (Order..*.) Order.le <$>
zipWithM compareTerm' (map unArg ts) (map (notMasked . namedArg) ps)

@andreasabel, can you take a look at this? Do the lists necessarily have the same length? Or should we return Order.unknown more often?

@nad nad added the termination Issues relating to the termination checker label Aug 30, 2022
@nad nad added this to the 2.6.3 milestone Aug 30, 2022
@andreasabel
Copy link
Member

@nad wrote:

Do the lists necessarily have the same length?

No, ps can be longer as ts as the comment says.

Can you break it? I cannot:

open import Agda.Builtin.Nat

data D : Set where
  c : Nat  Nat  D

mutual
  f : D  Set
  f (c (suc n) 0) = g (c n)
  f (c n (suc m)) = f (c (suc n) m)

  g : (Nat  D)  Set
  g h = f (h 42)
Termination checking failed for the following functions:
  f
Problematic calls:
  f (c (suc n) m)

@nad nad added the type: bug Issues and pull requests about actual bugs label Aug 31, 2022
@andreasabel
Copy link
Member

{-# OPTIONS --termination-depth=2 #-}

open import Agda.Builtin.Nat
open import Agda.Builtin.Equality

data D : Set where
  c : Nat  Nat  D

mutual
  f : (x : D)  Nat
  f (c (suc n) 0) = g (c n)
  f (c n (suc (suc m))) = f (c (suc n) m)
  f _ = zero

  g : (h : Nat  D)  Nat
  g h = f (h 2)

loop : f (c 1 0) ≡ zero
loop = refl

Loops.

@andreasabel
Copy link
Member

andreasabel commented Aug 31, 2022

And leads to inconsistency:

{-# OPTIONS --termination-depth=2 #-}

data  : Set where
record  : Set where

data Nat : Set where
  zero : Nat
  suc  : Nat  Nat

{-# BUILTIN NATURAL Nat #-}

data _≡_ (A : Set) : Set  Set₁ where
  refl : A ≡ A

cast : {A B : Set}  A ≡ B  A  B
cast refl x = x

cast⁻¹ : {A B : Set}  A ≡ B  B  A
cast⁻¹ refl x = x

-- Exploit the termination checker bug to prove ⊥.

data D : Set where
  _,_ : Nat  Nat  D

P : D  Set
P (suc _ , _)           = ⊥
P (_     , suc (suc _)) = ⊥
P (0 , 0) = ⊤
P (0 , 1) =-- This does not hold definitionally (no exact split), so prove it.
lem :  n {m}  P (n , suc (suc m)) ≡ ⊥
lem 0       = refl
lem (suc n) = refl

mutual
  f : (x : D)  P x
  -- The loop:
  f (suc n , _)           = cast   (lem n) (g (n ,_))       -- this call to g is fishy!
  f (n     , suc (suc m)) = cast⁻¹ (lem n) (f (suc n , m))
  -- Base cases:
  f (0 , 0) = record{}
  f (0 , 1) = record{}

  g : (h : Nat  D)  P (h 2)
  g h = f (h 2)

loop : ⊥
loop = f (1 , 0)

I can trace this issue back to at least Agda 2.4.2.4 (my oldest Agda).

@andreasabel andreasabel added the false Proof of the empty type which checks without known-unsafe flags (e.g. without --type-in-type) label Aug 31, 2022
andreasabel added a commit that referenced this issue Aug 31, 2022
Underapplied constructor terms are now unrelated to constructor
patterns in the termination order.
andreasabel added a commit that referenced this issue Aug 31, 2022
Underapplied constructor terms are now unrelated to constructor
patterns in the termination order.
@nad
Copy link
Contributor Author

nad commented Sep 1, 2022

The use of zipWithM dates to 2013 (aa2eb80).

@andreasabel
Copy link
Member

andreasabel commented Sep 1, 2022

Yeah, now one stumbled over this in 9 years, given the counterexample is contrived and requires --termination-depth=2.

@andreasabel andreasabel changed the title Strange code in the termination checker Non-terminating function over tuples passed with --termination-depth=2 Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false Proof of the empty type which checks without known-unsafe flags (e.g. without --type-in-type) termination Issues relating to the termination checker type: bug Issues and pull requests about actual bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants