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

Guardedness checker inconsistency with copatterns #1209

Closed
GoogleCodeExporter opened this issue Aug 8, 2015 · 25 comments
Closed

Guardedness checker inconsistency with copatterns #1209

GoogleCodeExporter opened this issue Aug 8, 2015 · 25 comments
Assignees
Labels
coinduction Coinductive records, musical coinduction copatterns Definitions by copattern matching: projections on the LHS false Proof of the empty type which checks without known-unsafe flags (e.g. without --type-in-type) sized types Sized types, termination checking with sized types, size inference termination Issues relating to the termination checker type: bug Issues and pull requests about actual bugs
Milestone

Comments

@GoogleCodeExporter
Copy link

Bug found by Dmitriy Traytel.

{-# OPTIONS --sized-types --copatterns #-}
module False where

open import Size

record Stream (A : Set) : Set where
  coinductive
  field
    shd : A
    stl : Stream A
open Stream public

map :  {A B : Set}  (A  B)  Stream A  Stream B
shd (map f a) = f (shd a)
stl (map f a) = map f (stl a)

data Mu-Stream : Size  Set where
  inn :  {i}  Stream (Mu-Stream i)  Mu-Stream (↑ i)

iter :  {A}  (Stream A  A)   {i}  Mu-Stream i  A
iter s (inn t) = s (map (iter s) t)

wit : Stream (Mu-Stream ∞)
shd wit = inn wit  -- SHOULD BE REJECTED
stl wit = wit

data  : Set where

false : ⊥
false = iter shd (inn wit)

It is a bug in the untyped guardedness check.

  wit : Stream (Mu-Stream ∞)
  shd wit = inn wit
  stl wit = wit

Since (shd wit) is not a recursive position for streams, the recursive call to
wit should be banned here. Only (stl wit) is a recursive position.

Original issue reported on code.google.com by andreas....@gmail.com on 19 Jun 2014 at 6:46

@GoogleCodeExporter
Copy link
Author

If termination checking was on sizes only, wit would not go through:

{-# OPTIONS --sized-types --copatterns #-}
module _ where

open import Common.Size

record Stream (A : Set) {i : Size} : Set where
  coinductive
  field
    head : A
    tail : {j : Size< i}  Stream A {j}
open Stream public

data Mu-Stream : Size  Set where
  inn :  {i}  Stream (Mu-Stream i)  Mu-Stream (↑ i)

wit : {i}  Stream (Mu-Stream ∞) {i}
head (wit {i}) = inn (wit {∞})
tail (wit {i}) {j} = wit {j}

Original comment by andreas....@gmail.com on 19 Jun 2014 at 6:54

@GoogleCodeExporter
Copy link
Author

GoogleCodeExporter commented Aug 8, 2015

Fixed by 66dfc9e with test case 150cfbe.

Original comment by andreas....@gmail.com on 1 Jul 2014 at 2:23

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Original comment by andres.s...@gmail.com on 18 Jul 2014 at 5:36

  • Added labels: Milestone-2.4.0.2

@GoogleCodeExporter GoogleCodeExporter added type: bug Issues and pull requests about actual bugs auto-migrated termination Issues relating to the termination checker coinduction Coinductive records, musical coinduction sized types Sized types, termination checking with sized types, size inference copatterns Definitions by copattern matching: projections on the LHS labels Aug 8, 2015
@GoogleCodeExporter
Copy link
Author

By playing around with test/fail/Issue1209.agda with Pierre Hyvernat, I found a
slight variation brings the issue back:

-- Andreas, 2014-01-07 Issue reported by Dmitriy Traytel
-- Andreas, 2015-04-15 Issue resurrected with Pierre Hyvernat

{-# OPTIONS --copatterns #-}

module _ where

open import Common.Size
open import Common.Prelude
open import Common.Product

record Stream (A : Set) : Set where
  coinductive
  field
    force : A × Stream A
open Stream

map :  {A B}  (A  B)  Stream A  Stream B
force (map f s) = let  a , as = force s  in  f a , map f as

-- This type should be empty, as Stream A is isomorphic to ℕ → A.
data D : (i : Size)  Set where
  lim :  i  Stream (D i)  D (↑ i)

-- Emptiness witness for D.
empty :  i  D i  ⊥
empty .(↑ i) (lim i s) = empty i (proj₁ (force s))

-- BAD: But we can construct an inhabitant.
inh : Stream (D ∞)
force inh = lim ∞ inh , inh -- Should be rejected by termination checker.

absurd : ⊥
absurd = empty ∞ (lim ∞ inh)

Original comment by andreas....@gmail.com on 15 Apr 2015 at 9:28

  • Changed state: Accepted
  • Removed labels: Milestone-2.4.0.2

@GoogleCodeExporter
Copy link
Author

Same problem for musical coinduction

open import Common.Coinduction renaming (∞ to Delay)
open import Common.Size
open import Common.Product

data  : Set where

record Stream (A : Set) : Set where
  inductive
  constructor delay
  field
    force : Delay (A × Stream A)
open Stream

head : {A}  Stream A  A
head s = proj₁ (♭ (force s))

-- This type should be empty, as Stream A is isomorphic to ℕ → A.
data D : (i : Size)  Set where
  lim :  i  Stream (D i)  D (↑ i)

-- Emptiness witness for D.
empty :  i  D i  ⊥
empty .(↑ i) (lim i s) = empty i (head s)

-- BAD: But we can construct an inhabitant.
inh : Stream (D ∞)
inh = delay (♯ (lim ∞ inh , inh)) -- Should be rejected by termination checker.

absurd : ⊥
absurd = empty ∞ (lim ∞ inh)

Original comment by andreas....@gmail.com on 15 Jul 2015 at 7:59

  • Added labels: Coinduction, SizedTypes

@GoogleCodeExporter
Copy link
Author

See also https://lists.chalmers.se/pipermail/agda/2011/003339.html

-- npouillard, 2011-10-04

open import Coinduction renaming (∞ to co)
open import Size

data  : Set where

data Mu-co : Size  Set where
  inn :  {i}  co (Mu-co i)  Mu-co (↑ i)

iter :  {i}  Mu-co i  ⊥
iter (inn t) = iter (♭ t)

bla : Mu-co ∞
bla = inn (♯ bla)

false : ⊥
false = iter bla

Original comment by andreas....@gmail.com on 15 Jul 2015 at 8:10

@GoogleCodeExporter
Copy link
Author

This variant is due to Nisse, I think:

{-# OPTIONS --copatterns #-}

open import Size
open import Data.Product

data  : Set where

record Delay (A : Set) : Set where
  coinductive
  constructor 
  field ♭   : A × (⊥  Delay A)  -- Make it recursive to please termination checker
open Delay

data D : Size  Set where
  inn :  i  Delay (D i)  D (↑ i)

iter : {i}  D i  ⊥
iter (inn i t) = iter (proj₁ (♭ t))

-- Should be rejected by termination checker
bla : Delay (D ∞)
♭ bla = inn ∞ bla , λ()

false : ⊥
false = iter (proj₁ (♭ bla))

Original comment by andreas....@gmail.com on 15 Jul 2015 at 8:46

@GoogleCodeExporter
Copy link
Author

Version with mutual definition:

{-# OPTIONS --copatterns #-}

open import Size

data  : Set where

mutual
  data D : Size  Set where
    inn :  i  D' i  D (↑ i)

  record D' (i : Size) : Set where
    coinductive
    constructor 
    field ♭   : D i
open D'


iter : {i}  D i  ⊥
iter (inn i t) = iter (♭ t)

-- Should be rejected by termination checker
bla : D' ∞
♭ bla = inn ∞ bla

false : ⊥
false = iter (♭ bla)

Original comment by andreas....@gmail.com on 15 Jul 2015 at 8:57

@GoogleCodeExporter
Copy link
Author

The symmetric version is correctly rejected, as coinductive projections are not size-preserving. Maybe we should just make inductive constructors not guardedness-preserving.
Only that then copatterns are in general only usable with sizes.

{-# OPTIONS --copatterns #-}

open import Size

data  : Set where

mutual
  data D (i : Size) : Set where
    inn : D' i  D i

  record D' (i : Size) : Set where
    coinductive
    field ♭   : {j : Size< i}  D j
open D'

bla : {i}  D' i
♭ bla = inn bla

-- Should be rejected by termination checker
iter : D ∞  ⊥
iter (inn t) = iter (♭ t)

false : ⊥
false = iter (♭ bla)

Original comment by andreas....@gmail.com on 15 Jul 2015 at 9:07

@jespercockx jespercockx added the false Proof of the empty type which checks without known-unsafe flags (e.g. without --type-in-type) label May 16, 2017
@jespercockx
Copy link
Member

I guess this deserves the false label?

@nad
Copy link
Contributor

nad commented Jan 29, 2018

Maybe we should just make inductive constructors not guardedness-preserving.
Only that then copatterns are in general only usable with sizes.

Perhaps we could hide the old guardedness machinery under a flag:

           --guardedness                               enable constructor-based guarded corecursion (inconsistent with --sized-types)
           --guardedness-preserving-type-constructors  treat type constructors as inductive constructors when checking productivity, implies --guardedness
           --sized-types                               use sized types (default, inconsistent with --guardedness)
           --no-sized-types                            disable sized types

We should reject the combination of --guardedness and --sized-types (or the absence of --no-sized-types) when --safe is active. Perhaps other combinations should also be rejected.

@UlfNorell UlfNorell added this to the 2.5.5 milestone May 27, 2018
@jespercockx
Copy link
Member

I agree, the combination of --guardedness, --sized-types, and --safe should be rejected. Maybe we should also make --sized-types imply --no-guardedness by default (but still allow the user to override it with an explicit --guardedness flag).

@nad
Copy link
Contributor

nad commented Nov 14, 2018

I think the default should be something that we believe is safe, to avoid leading people astray.

@nad nad added this to the 2.6.0 milestone Dec 12, 2018
@nad
Copy link
Contributor

nad commented Dec 13, 2018

Blocked by #3451.

@nad
Copy link
Contributor

nad commented Dec 13, 2018

I tried to fix this by (basically) replacing guardedness with Order.Unknown in the following line when --guardedness is not active:

return $ addGuardedness guardedness (size es) (size pats) matrix

Is this all that is needed? The test suite points to a potential problem. For instance, take Issue1375.agda. The following is the previous error message:

Issue1375.agda:5,1-17,25
Termination checking failed for the following functions:
Type
Problematic calls:
Type
(at Issue1375.agda:9,12-16)
Term type
(at Issue1375.agda:11,10-14)
type
(at Issue1375.agda:11,15-19)
Unsolved interaction metas at the following locations:
Issue1375.agda:16,34-38
Issue1375.agda:16,52-56
Issue1375.agda:17,21-25

The new error message does not include the problematic call to type. Does anyone know why?

@nad
Copy link
Contributor

nad commented Dec 17, 2018

@andreasabel?

@nad
Copy link
Contributor

nad commented Dec 18, 2018

The standard library mixes the two forms of corecursion. For instance, Codata.Conat provides conversions to and from Codata.Musical.Conat.Coℕ. @MatthewDaggitt and @gallais, how would you like to address this? It is possible to activate both constructor-based guarded corecursion and corecursion based on sized types in one module, but not if --safe is active.

@nad
Copy link
Contributor

nad commented Dec 18, 2018

Note also that --sized-types and --guardedness will presumably both become infective (#2487).

@nad
Copy link
Contributor

nad commented Dec 18, 2018

Some further questions:

  • Should the musical builtins be available when --guardedness is not active?

  • Should it be possible to define coinductive records that do not use sized types without turning on --guardedness? (In that case: Is there a correct way to "use sized types"?)

  • Should both --sized-types and --guardedness be off by default?

    Given that we plan to fix the problems with sized types I suggest that we let sized types be on by default unless --safe is used, in which case neither option is activated. Later we can perhaps make --sized-types the default also when --safe is in use.

@gallais
Copy link
Member

gallais commented Dec 18, 2018

@MatthewDaggitt and @gallais, how would you like to address this?

We could move the conversion functions to the Codata.Musical.* modules. They exist
solely because the (unsafe) IO currently uses old-style coinduction and we want to be
able to write programs using the new-style one.

@nad nad closed this as completed in e205a43 Dec 19, 2018
@nad
Copy link
Contributor

nad commented Dec 19, 2018

I've pushed a fix now. Some notes:

  • I moved the conversion functions to the Musical modules. I also changed Data.Container.ν so that it uses the sized variant of M. I pushed these changes to the experimental branch.

  • I did not address the further questions above.

  • I discussed the matter of missing problematic calls with @andreasabel, and I got the impression that he did not think that this is a sign of a (new) hole in the termination checker.

@nad
Copy link
Contributor

nad commented Dec 19, 2018

Blocked by #3451.

Instead of waiting for a fix I pushed a test case to test/Succeed instead of test/Fail.

@nad
Copy link
Contributor

nad commented Dec 20, 2018

@andreasabel, consider the following code:

Type : Set

postulate
  type : Type
  Term : Type  Set

Type = Term type

When I run Agda with -vterm.found.call:20 I get the following output:

found call from Type to type
found call from Type to Term
found call from type to type
found call from type to Term
found call from Term to Type

This looks strange to me. Why are there calls from type to type/Term, but no call from type to Type?

@nad
Copy link
Contributor

nad commented Dec 20, 2018

I'm moving the discussion of the potential problem discussed in the previous comment to #1556.

@andreasabel
Copy link
Member

Your fix as described in #1209 (comment) LGTM, @nad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coinduction Coinductive records, musical coinduction copatterns Definitions by copattern matching: projections on the LHS false Proof of the empty type which checks without known-unsafe flags (e.g. without --type-in-type) sized types Sized types, termination checking with sized types, size inference termination Issues relating to the termination checker type: bug Issues and pull requests about actual bugs
Projects
None yet
Development

No branches or pull requests

6 participants