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

Instance Search breaks Termination Highlighting #5875

Closed
lawcho opened this issue Apr 27, 2022 · 4 comments · Fixed by #5881
Closed

Instance Search breaks Termination Highlighting #5875

lawcho opened this issue Apr 27, 2022 · 4 comments · Fixed by #5881
Assignees
Labels
meta Metavariables, insertion of implicit arguments, etc range regression in 2.6.0 Regression that first appeared in Agda 2.6.0 termination Issues relating to the termination checker ux: error reporting Issues to do with how Agda reports errors
Milestone

Comments

@lawcho
Copy link
Contributor

lawcho commented Apr 27, 2022

The termination checker correctly finds the bad call in the following code:

record Monad-Lite (M : Set  Set) : Set₁ where
    field _>>=_ : {A B}  M A  (A  M B)  M B

instance
    Monad-id : Monad-Lite (λ x  x)
    Monad-id .Monad-Lite._>>=_ x f = f x

open Monad-Lite Monad-id

data Unit : Set where
    tt : Unit

loop : Unit
loop =
    loop >>= λ x  x

However, when I replace open Monad-Lite Monad-id with open Monad-Lite {{...}}, the highlighting disappears.

The termination checker still complains, it just doesn't say where the suspicious call is any more.

Screenshots:

Expected Behaviour

Missing Highlighting

Notes:

  • Tested with Agda versions 2.6.2.1 and 2.6.2.2
  • Tested with agda-mode plugins for both emacs and VSCode
@andreasabel andreasabel added termination Issues relating to the termination checker ux: error reporting Issues to do with how Agda reports errors regression in 2.6.0 Regression that first appeared in Agda 2.6.0 labels Apr 27, 2022
@andreasabel
Copy link
Member

Thanks for reporting! This used to work in Agda 2.5.2/3/4, broke in 2.6.0.

@andreasabel
Copy link
Member

Starting bisection:

agda-bisect --with-cabal=cabal-3.2 -w ghc-8.4.4 --good v2.5.4 --bad v2.6.0 --script $PWD/issue5875.sh

Skript issue5875.sh:

$1 Issue5875b.agda | grep "(at"

The file Issue5875b.agda is the MWE from the OP.

@andreasabel andreasabel self-assigned this Apr 28, 2022
@andreasabel andreasabel added this to the 2.6.3 milestone Apr 28, 2022
@andreasabel
Copy link
Member

Bisection produced this:
47358b3 is the first bad commit
Date: Thu Dec 13 13:32:18 2018 +0100
[ #3435 ] postpone instance constraints while checking applications

Let's see how helpful this information is to fix the issue...

andreasabel referenced this issue Apr 28, 2022
* killrange when solving metas

In order to avoid positivity issues to be reported with the range
of the term that made solving the meta possible rather than where
the meta actually originated from. As a consequence a few golden
valus are changed.

* Keeping the record's tele's ranges

In order to be able to report positivity issues, we need to keep
the ranges in the record's telescope.
@andreasabel
Copy link
Member

The real culprit seems to be 2970da8 (ping @gallais) see 2970da8#r72426685:

[ warning ] Highlighting positivity issues

  • killrange when solving metas
    In order to avoid positivity issues to be reported with the range of the term that made solving the meta possible rather than where the meta actually originated from. As a consequence a few golden valu[e]s are changed.

andreasabel added a commit that referenced this issue Apr 28, 2022
This brings back termination error locations lost in
2970da8.

It also brings back a maybe unwanted positivity highlighting that was
removed by cited commit.  However, I think highlighting THIS
```
data H : Set where
  inH : ∀ f → f ≡ (λ (x : H) → x) -> H -- non-positive occurrence is
                                       -- in the implicit type of f
                       -- ^ THIS
```
isn't so wrong.  Maybe there are more convincing examples...
andreasabel added a commit that referenced this issue Apr 28, 2022
This brings back termination error locations lost in
2970da8.

It also brings back a maybe unwanted positivity highlighting that was
removed by cited commit.  However, I think highlighting THIS
```
data H : Set where
  inH : ∀ f → f ≡ (λ (x : H) → x) -> H -- non-positive occurrence is
                                       -- in the implicit type of f
                       -- ^ THIS
```
isn't so wrong.  Maybe there are more convincing examples...
@andreasabel andreasabel added meta Metavariables, insertion of implicit arguments, etc range labels Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Metavariables, insertion of implicit arguments, etc range regression in 2.6.0 Regression that first appeared in Agda 2.6.0 termination Issues relating to the termination checker ux: error reporting Issues to do with how Agda reports errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants