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

Naked type application panics #962

Closed
yav opened this issue Nov 13, 2020 · 4 comments
Closed

Naked type application panics #962

yav opened this issue Nov 13, 2020 · 4 comments
Assignees
Labels
bug Something not working correctly low-hanging fruit For issues that should be easy to fix parser Issues with lexing or parsing.
Milestone

Comments

@yav
Copy link
Member

yav commented Nov 13, 2020

I got confused with the notation for demonting types and wrote a type application instead, which caused a panic:

Loading module Cryptol
Cryptol> `{1}
cryptol: You have encountered a bug in Cryptol's implementation.
*** Please create an issue at https://github.com/GaloisInc/cryptol/issues

%< --------------------------------------------------- 
  Revision:  a7d352b585507a74440968b3cec46f40b8a047af
  Branch:    master (uncommited files present)
  Location:  doCheckType
  Message:   TTyApp found when kind checking, but it should have been eliminated already
CallStack (from HasCallStack):
  panic, called at src/Cryptol/Utils/Panic.hs:21:9 in cryptol-2.9.1.99-inplace:Cryptol.Utils.Panic
  panic, called at src/Cryptol/TypeCheck/Kind.hs:391:24 in cryptol-2.9.1.99-inplace:Cryptol.TypeCheck.Kind
%< --------------------------------------------------- 
@yav yav added the bug Something not working correctly label Nov 13, 2020
@yav yav self-assigned this Nov 13, 2020
@brianhuffman
Copy link
Contributor

Weird! Why does that even parse?

@robdockins
Copy link
Contributor

robdockins commented Nov 13, 2020

I think it's one of these strange cases where the parser makes `{ stuff } an expression form and fixes it up later to avoid LALR limitations. The mkEApp operator is expected to munge things around and remove the TTyApp forms.

@brianhuffman
Copy link
Contributor

It seems like whatever post-processing pass we do over the parser AST that deals with TTyApp should only allow type applications to identifiers. Currently cryptol is totally fine with empty type applications anywhere you want, like this:

Cryptol> :t (((True`{}, zero`{Integer})`{}).1)`{}
(((True`{}, zero`{Integer})`{}).1)`{} : Integer

Cryptol also is happy to have multiple adjacent type applications, possibly with some of them empty; the effect is like all the applications are appended together:

Cryptol> :t number`{3}`{}`{}`{}`{}`{Integer}
number`{3}`{}`{}`{}`{}`{Integer} : Integer

I think it would be reasonable to forbid stacked type applications, as you can always just combine them into one like number`{3, Integer}.

@yav yav removed their assignment Nov 13, 2020
@yav
Copy link
Member Author

yav commented Nov 13, 2020

I unassigned myself for the moment, as I am working on a different part of the code. I'll revisit this if no one else has picked it up in the mean time.

@brianhuffman brianhuffman added parser Issues with lexing or parsing. low-hanging fruit For issues that should be easy to fix labels Nov 13, 2020
@atomb atomb added this to the 2.11.0 milestone Dec 11, 2020
This was referenced Jan 25, 2021
robdockins added a commit that referenced this issue Feb 10, 2021
We now reject situations where the user writes a type application
on a term that is not an identifier, and we correctly unwind
and reapply tuple and field selectors.

Fixes #962
Fixes #1045
Fixes #1050
@robdockins robdockins self-assigned this Feb 12, 2021
robdockins added a commit that referenced this issue Feb 16, 2021
We now reject situations where the user writes a type application
on a term that is not an identifier, and we correctly unwind
and reapply tuple and field selectors.

Fixes #962
Fixes #1045
Fixes #1050
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something not working correctly low-hanging fruit For issues that should be easy to fix parser Issues with lexing or parsing.
Projects
None yet
Development

No branches or pull requests

4 participants