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

WIP: Copy Covariance from Arend( #851

Closed
wants to merge 13 commits into from
Closed

WIP: Copy Covariance from Arend( #851

wants to merge 13 commits into from

Conversation

mio-19
Copy link
Contributor

@mio-19 mio-19 commented Dec 23, 2022

fix #849

@codecov
Copy link

codecov bot commented Dec 23, 2022

Codecov Report

Merging #851 (7f3d97b) into main (6c6215d) will decrease coverage by 0.86%.
The diff coverage is 47.23%.

@@             Coverage Diff              @@
##               main     #851      +/-   ##
============================================
- Coverage     82.69%   81.83%   -0.87%     
- Complexity     3295     3319      +24     
============================================
  Files           279      287       +8     
  Lines         10332    10545     +213     
  Branches       1226     1263      +37     
============================================
+ Hits           8544     8629      +85     
- Misses         1118     1241     +123     
- Partials        670      675       +5     
Impacted Files Coverage Δ
...ain/java/org/aya/resolve/context/ModuleExport.java 89.09% <ø> (ø)
...ava/org/aya/tyck/covariance/CovarianceChecker.java 0.00% <0.00%> (ø)
...a/tyck/covariance/ParametersCovarianceChecker.java 0.00% <0.00%> (ø)
.../org/aya/tyck/covariance/RecursiveDataChecker.java 0.00% <0.00%> (ø)
.../java/org/aya/tyck/error/NonPositiveDataError.java 0.00% <0.00%> (ø)
.../src/main/java/org/aya/concrete/stmt/TeleDecl.java 86.58% <20.00%> (-9.19%) ⬇️
...rc/main/java/org/aya/cli/parse/ModifierParser.java 87.14% <87.14%> (ø)
...src/main/java/org/aya/cli/parse/AyaGKProducer.java 90.47% <94.44%> (-0.12%) ⬇️
...e/src/main/java/org/aya/concrete/stmt/UseHide.java 100.00% <100.00%> (ø)
...g/aya/cli/parse/error/ContradictModifierError.java 100.00% <100.00%> (ø)
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ice1000
Copy link
Member

ice1000 commented Dec 23, 2022

I still think the error message is not very helpful. Is there a way to show the exact expression that is problematic?

Ideally, I would appreciate an error message that can help you understand what's wrong in the type checker without debugging the source code of the compiler.

@mio-19
Copy link
Contributor Author

mio-19 commented Dec 23, 2022

I still think the error message is not very helpful. Is there a way to show the exact expression that is problematic?

Ideally, I would appreciate an error message that can help you understand what's wrong in the type checker without debugging the source code of the compiler.

I think sourcepos of the ctor definition is enough for user to understand what's wrong in most cases

@ice1000
Copy link
Member

ice1000 commented Dec 23, 2022

I still think the error message is not very helpful. Is there a way to show the exact expression that is problematic?

Ideally, I would appreciate an error message that can help you understand what's wrong in the type checker without debugging the source code of the compiler.

I think sourcepos of the ctor definition is enough for user to understand what's wrong in most cases

What if there are 3 mutually recursive data types with each of them 2-10 constructors?

Aya is going to self host and the definition of Term will be inductive-inductively defined together with contexts and type formers. Every introduction and elimination rule is a constructor. This is a real-world example.

@mio-19
Copy link
Contributor Author

mio-19 commented Dec 23, 2022

What if there are 3 mutually recursive data types with each of them 2-10 constructors?

I think it still should be a very rare case to trigger an error and see a hardly understandable error message.

might implement more detailed error message in future prs

@ice1000
Copy link
Member

ice1000 commented Dec 23, 2022

What if there are 3 mutually recursive data types with each of them 2-10 constructors?

I think it still should be a very rare case to trigger an error and see a hardly understandable error message.

might implement more detailed error message in future prs

It's exactly when the code gets complicated do people need to look at error messages. So, I would prefer higher quality ones.

@ice1000
Copy link
Member

ice1000 commented Jan 1, 2023

I think positivity with the presence of HITs will be more complex. See agda/agda#6336

@ice1000 ice1000 closed this Jan 1, 2023
@ice1000 ice1000 deleted the Covariance branch January 1, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Positivity check is incorrectly implemented
2 participants