Skip to content

Check typing flags in private constants.#21880

Open
ppedrot wants to merge 1 commit intorocq-prover:masterfrom
ppedrot:fix-20550
Open

Check typing flags in private constants.#21880
ppedrot wants to merge 1 commit intorocq-prover:masterfrom
ppedrot:fix-20550

Conversation

@ppedrot
Copy link
Copy Markdown
Member

@ppedrot ppedrot commented Apr 2, 2026

We are a bit laxer than just requiring the flags to be kept unchanged across environments, as we also allow for "stricter" flags.

Fixes #20550: Side effects lose track of typing flags leading to inconsistency.

I do not know how to write a test for this though, the Qed failure is seemingly not caught by the Fail wrapper and propagates to toplevel regardless of it. Any idea?

We are a bit laxer than just requiring the flags to be kept unchanged across
environments, as we also allow for "stricter" flags.

Fixes rocq-prover#20550: Side effects lose track of typing flags leading to inconsistency.
@ppedrot ppedrot added this to the 9.3+rc1 milestone Apr 2, 2026
@ppedrot ppedrot requested a review from a team as a code owner April 2, 2026 15:39
@ppedrot ppedrot added kind: fix This fixes a bug or incorrect documentation. request: full CI Use this label when you want your next push to trigger a full CI. labels Apr 2, 2026
@coqbot-app coqbot-app bot removed the request: full CI Use this label when you want your next push to trigger a full CI. label Apr 2, 2026
@JasonGross
Copy link
Copy Markdown
Member

I do not know how to write a test for this though, the Qed failure is seemingly not caught by the Fail wrapper and propagates to toplevel regardless of it. Any idea?

Use Fail abstract or Control.case?

@JasonGross
Copy link
Copy Markdown
Member

Oh, sorry, I didn't realize that this was exactly testing an issue with abstract

sprop_allowed = sprop_allowed1;
allow_uip = allow_uip1;
(* The flags below do not change the theory *)
conv_oracle = _;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to enable warning 9 if you want to ensure the field list is exhaustive.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't it part of the default flags at some point?

@SkySkimmer
Copy link
Copy Markdown
Contributor

I do not know how to write a test for this though, the Qed failure is seemingly not caught by the Fail wrapper and propagates to toplevel regardless of it. Any idea?

The problem seems to be that Fail (or Succeed) Qed runs with the global env of the proof, which contains the private constants, so

if Environ.mem_constant e.seff_constant env then None
says "nothing to check, just reuse the constant".

You can make an output test I guess.
Ultimately we probably want abstract to use the local flags, then the test will Fail abstract instead of failing at Qed (and we would need some plugin to test the kernel check).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: fix This fixes a bug or incorrect documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Side effects lose track of typing flags leading to inconsistency

3 participants