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

Weird warning for instance declarations of bad type #6379

Closed
felixwellen opened this issue Dec 3, 2022 · 5 comments · Fixed by #6381
Closed

Weird warning for instance declarations of bad type #6379

felixwellen opened this issue Dec 3, 2022 · 5 comments · Fixed by #6381
Assignees
Labels
instance Instance resolution PR welcome Welcome to submit a PR fixing this issue ux: documentation Issues relating to Agda's documentation ux: error reporting Issues to do with how Agda reports errors
Milestone

Comments

@felixwellen
Copy link
Contributor

When writing something like this:

instance _ = Set

I get the following error message:

Terms marked as eligible for instance search should end with a
name, so `instance' is ignored here.
when checking the definition of _

It should be more helpful to write more or less, what the manual says:

Terms marked as eligible for instance search should be of type {Γ} → C vs, 
where C is a postulated name, a bound variable, or the name of a data or record type [...]
@felixwellen felixwellen added the instance Instance resolution label Dec 3, 2022
@andreasabel
Copy link
Member

... where C is a postulated name, a bound variable,

More precise:

where C evaluates to a postulated name...

I think bound variable is no longer a valid form for instances. That should also be fixed in the docs.

@andreasabel andreasabel added ux: error reporting Issues to do with how Agda reports errors ux: documentation Issues relating to Agda's documentation PR welcome Welcome to submit a PR fixing this issue labels Dec 4, 2022
@andreasabel andreasabel added this to the 2.6.4 milestone Dec 4, 2022
@felixwellen felixwellen self-assigned this Dec 4, 2022
@felixwellen
Copy link
Contributor Author

I think bound variable is no longer a valid form for instances. That should also be fixed in the docs.

Yes:

addTypedInstance x t = do
  (tel , n) <- getOutputTypeName t
  case n of
    OutputTypeName n -> addNamedInstance x n
    OutputTypeNameNotYetKnown{} -> addUnknownInstance x
    NoOutputTypeName -> warning $ WrongInstanceDeclaration
    OutputTypeVar -> warning $ WrongInstanceDeclaration
    OutputTypeVisiblePi -> warning $ InstanceWithExplicitArg x

@felixwellen
Copy link
Contributor Author

felixwellen commented Dec 4, 2022

I'm a bit confused that the following checks without warning:

module _ (B : Set) where
  use : {{t : B}}  B
  use {{t}} = t

but when we declare an instance of the type, there is a warning:

module _ (B : Set) where
  postulate b : B
  instance _ = b
  use : {{t : B}}  B
  use {{t}} = t

produces:

Terms marked as eligible for instance search should be of type {Γ}
→ C vs, where C evaluates to a postulated name or the name of a
data or record type, so `instance' is ignored here.
when checking the definition of _

(That's already the new warning message from #6381)

@felixwellen
Copy link
Contributor Author

Never mind everyone, I got it and have to update #6381 ...

@felixwellen
Copy link
Contributor Author

For instance declarations, I would stick with the warning message above. For the types of instance arguments, I think the following makes sense (it is at least something I would have understood):

InstanceNoOutputTypeName b -> fsep $
      pwords "Instance arguments whose type is not {Γ} → C vs, where C evaluates to a variable or the name of a data or record type, are never considered by instance search," ++
      pwords "so having an instance argument" ++ [return b] ++ pwords "has no effect."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
instance Instance resolution PR welcome Welcome to submit a PR fixing this issue ux: documentation Issues relating to Agda's documentation 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