agda / agda

Agda is a dependently typed programming language / interactive theorem prover.
https://wiki.portal.chalmers.se/agda/pmwiki.php
Other
2.41k stars 339 forks source link

Bad interaction between instance search and opaque #7304

Closed UlfNorell closed 3 weeks ago

UlfNorell commented 3 weeks ago

Instances defined outside the opaque block sometimes fail to resolve when inside. Possibly some issue with the discrimination trees (ping @plt-amy). Works on 2.6.4.3.

it : {A : Set} → ⦃ A ⦄ → A
it ⦃ x ⦄ = x

postulate
  X : Set
  C : Set → Set
  c : {A : Set} → ⦃ C A ⦄ → A

opaque
  Y : Set
  Y = X

instance
  postulate CY : C Y

opaque
  unfolding Y

  works : Y
  works = c ⦃ it ⦄

  fails : Y
  fails = c
UlfNorell commented 3 weeks ago

Bisection blames 403ee4263 (#7183).

plt-amy commented 3 weeks ago

(Not at home today, I'll only be able to look into this tomorrow.)

plt-amy commented 3 weeks ago

Well, I can't look into it properly, but I think removing this check, so it always does ignoreAbstractMode, should fix this issue.

UlfNorell commented 3 weeks ago

Thanks! It makes the example go through. I'll make a PR and see if anything breaks.

UlfNorell commented 3 weeks ago

(Cross-post to the PR). The attempted fix breaks this example

module _ where

open import Agda.Builtin.Nat
open import Agda.Builtin.List

it : {A : Set} → ⦃ A ⦄ → A
it ⦃ x ⦄ = x

opaque
  X : Set
  X = Nat

Number = Nat
record Count (A : Set) : Set where
  field count : A → Number

open Count ⦃...⦄ public

instance
  postulate Count-X : Count X

works : X → Nat
works s = count ⦃ it ⦄ s

opaque
  unfolding X
  works₂ : X → Nat
  works₂ s = count s

fails : X → Nat
fails s = count s
plt-amy commented 3 weeks ago

Thanks for the minimised example (and for trying the fix), @UlfNorell! That makes sense, since by ignoring abstract when adding the instance we in effect add it for Count Nat instead. I think a more correct fix will be to treat opaque symbols as FlexKs (ie treat them as if they were blocked & could reduce, since they can, and leave it for instance selection to discard them, using the conversion checker), but that's not something I can implement on my phone; tomorrow, then.