-
Notifications
You must be signed in to change notification settings - Fork 682
"Warnings" and "Debug" options are synterp not interp #19981
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
Conversation
@coqbot run full ci |
🔴 CI failure at commit c5fed9e without any failure in the test-suite ✔️ Corresponding job for the base commit 03dd829 succeeded ❔ Ask me to try to extract a minimal test case that can be added to the test-suite 🏃
|
@coqbot ci minimize |
I have initiated minimization at commit c5fed9e for the suggested target ci-metacoq as requested. |
Error: Could not minimize file (from ci-metacoq) (full log on GitHub Actions, cc @JasonGross) build log (truncated to last 26KiB; full 5.2MiB file on GitHub Actions Artifacts under
|
c5fed9e
to
6f77387
Compare
There is a change of behaviour even in coqc: if |
Indeed, and applying to both synterp and interp would lead to similarly unexpected behaviors with the flags still being in a strange state at the beginning of the interp phase. |
This looks like there is some design to pin down, shouldn't this be postponed? |
I also think we'd better not rush the design. Postponing seems the reasonable thing to do then. |
Master is bugged IMO, it also makes UI behave differently from compilation |
Considering impact on CI seems minimal, ok by me for "the new bug is less bad than the current one" but we need at least a small changelog entry. |
Adapt to rocq-prover/rocq#19981 (Set Warnings is synterp phase)
6f77387
to
84f409c
Compare
done |
@@ -0,0 +1,5 @@ | |||
- **Fixed:** | |||
:opt:`Debug` and :opt:`Warnings` are classified as Synterp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a shame that #19759 is not merged yet, otherwise we could refer to it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cc @Zimmi48 FYI (but definitely not a serious/urgent issue)
@coqbot merge now |
@proux01: Please take care of the following overlays:
|
Overlays: