Skip to content

Fix #7643: coverage: handle blocked sort in isFibrant #7672

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

Merged
merged 2 commits into from
Jan 2, 2025
Merged

Conversation

andreasabel
Copy link
Member

@andreasabel andreasabel commented Jan 2, 2025

Fix #7643: coverage: handle blocked sort in isFibrant
WAS: uncaught pattern violation.
NOW: default to non-fibrant.

@andreasabel andreasabel added this to the 2.8.0 milestone Jan 2, 2025
@andreasabel andreasabel added coverage Pattern-matching coverage checker cubical Cubical Agda paraphernalia: Paths, Glue, partial elements, hcomp, transp internal-error Concerning internal errors of Agda labels Jan 2, 2025
@andreasabel andreasabel merged commit a27b403 into master Jan 2, 2025
27 checks passed
@jespercockx
Copy link
Member

Is defaulting the right thing to do here? In general, defaulting rules tend to make the result of elaboration dependent on the order in which constraints are solved and can result in brittle code as a result. As an alternative, we could add a constraint for postponing the call to checkCoverage so it could be retried once the fibrancy check is no longer blocked.

@andreasabel andreasabel deleted the is-fibrant branch January 3, 2025 18:54
@andreasabel
Copy link
Member Author

Yeah I guess one could be more careful here.

isFib <- fromRight (const False) <$> lift (isFibrant' t)
...
if isFib && (isHIT || not (null ixs)) && not (null mns) && inserttrailing == DoInsertTrailing
                   then computeHCompSplit delta1 n delta2 d pars ixs x tel ps cps

If we want to block on isFibrant' t we should make sure we only do it when we really want to computeHCompSplit, so, only if all the other conjuncts are true.
My current (lazy) solution is restorative for non-cubical stuff, but maybe a bit anti-Cubical.
In this regression-fix I didn't want to go full circle with a pattern-violation catching mechanism that the coverage checker does not seem to have yet, but I am open to move systematic solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coverage Pattern-matching coverage checker cubical Cubical Agda paraphernalia: Paths, Glue, partial elements, hcomp, transp internal-error Concerning internal errors of Agda regression in 2.6.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic: uncaught pattern violation
2 participants