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

deriving BEq fails for structures with Prop fields #3140

Closed
girving opened this issue Jan 4, 2024 · 8 comments · Fixed by #3191
Closed

deriving BEq fails for structures with Prop fields #3140

girving opened this issue Jan 4, 2024 · 8 comments · Fixed by #3191
Labels
bug Something isn't working

Comments

@girving
Copy link
Contributor

girving commented Jan 4, 2024

deriving BEq currently fails if a structure has Prop fields, a simple example being

structure S where
  p : True
  deriving BEq

which produces the error message

typeclass instance problem is stuck, it is often due to metavariables
  BEq (?m.302 x✝¹ x✝ a✝ b✝)

From discussion in Zulip and here, this is due to BEq being defined only over Type, not Sort. However, the desire is to keep it this way, and instead fix deriving BEq directly to ignore Prop fields (which is fine by proof irrelevance).

Impact

Add 👍 to issues you consider important. If others benefit from the changes in this proposal being added, please ask them to add 👍 to it.

@girving girving added the RFC Request for comments label Jan 4, 2024
@kmill
Copy link
Collaborator

kmill commented Jan 4, 2024

There's nothing preventing there from S having a BEq instance in principle, even a LawfulBEq. It would be a different story if it were structure S : Prop where ...

The DecidableEq derive handler works here, and from that you get a BEq:

structure S where
  p : True
  deriving DecidableEq

#synth BEq S -- succeeds

This seems to just be a bug in the BEq derive handler that it can't handle a Prop field in the structure. I think this RFC needs a better case for making BEq be parameterized over Sort _ rather than Type _ than just to avoid fixing the BEq deriving handler. (Thanks for filing this issue, though, since it's worth tracking the fact that the BEq derive handler has a bug!)

@girving
Copy link
Contributor Author

girving commented Jan 5, 2024

I’m happy to change this to be a bug report on deriving BEq if that’s the preference.

@girving girving changed the title RFC: Let BEq work for Props deriving BEq fails for structures with Prop fields Jan 5, 2024
@girving
Copy link
Contributor Author

girving commented Jan 5, 2024

I changed the title, but don’t have permission to change the label.

@kim-em kim-em added bug Something isn't working and removed RFC Request for comments labels Jan 8, 2024
@arademaker
Copy link

arademaker commented Jan 15, 2024

I got a similar errror in

structure Result where
 derivation : String
 mrs : MRS
deriving Repr

the error is

typeclass instance problem is stuck, it is often due to metavariables
  Repr (?m.3386 x✝ prec✝)

where MRS is defined as

structure MRS where
  top : Var
  index : Var
  preds : List EP
  hcons : List Constraint
  icons : List Constraint

instance : ToFormat MRS where
 format
 | {top := t, index := i, preds := ps, icons := [], hcons := hs} =>
   f!"[ LTOP: {t}
        INDEX: {i}
        RELS: < {Format.joinSep (ps.map fun a => format a) " "} >
        HCONS: < {Format.joinSep (hs.map fun a => format a) " "} > ]"
 | {top := t, index := i, preds := ps, icons := is, hcons := hs} =>
   f!"[ LTOP: {t}
        INDEX: {i}
        RELS: < {Format.joinSep (ps.map fun a => format a) " "} >
        HCONS: < {Format.joinSep (hs.map fun a => format a) " "} >
        ICONS: < {Format.joinSep (is.map fun a => format a) " "} > ]"

instance : Repr MRS where
 reprPrec m _ := f!"{m}"

@kim-em
Copy link
Collaborator

kim-em commented Jan 16, 2024

@girving, could you update the issue description? I think no need to keep the old version, it's just noise now. If you prefer to just open a new issue that might even be easier.

@arademaker
Copy link

From my side. Not sure how I solved, but the error disappeared after moving to Lean last release. Maybe my step by step procedure to update Lean and std were not ideally and some of the lake clean and lake build solve the issue.

@girving
Copy link
Contributor Author

girving commented Jan 17, 2024

@semorrison I updated the issue description.

github-merge-queue bot pushed a commit that referenced this issue Jan 18, 2024
Closes #3140

---------

Co-authored-by: Joachim Breitner <[email protected]>
@girving
Copy link
Contributor Author

girving commented Jan 18, 2024

Woohoo, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants