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

Remove bugged option MetaCoq Template Monad Debug. #1123

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

SkySkimmer
Copy link
Contributor

@SkySkimmer SkySkimmer commented Nov 25, 2024

It causes coq/vscoq#892 because optwrite is not supposed to be able to add persistent objects (libobject) to Coq's state but set_option_value does add one.

It's also bugged as
set_option_value ~stage:Interp (fun _ v -> v) key i sets the option to its current value v not a new value i. In other words Set MetaCoq Template Monad Debug. has no effect but its existence causes bugs.

Since nobody complained about it not working it's not worth fixing it instead of deleting it (also I'm not sure it can be fixed with the currently available APIs).

It causes coq/vscoq#892 because `optwrite`
is not supposed to be able to add persistent objects (libobject) to
Coq's state but `set_option_value` does add one.

It's also bugged as
`set_option_value ~stage:Interp (fun _ v -> v) key i`
sets the option to its current value not a new value.
In other words `Set MetaCoq Template Monad Debug.` has no effect but
its existence causes bugs.

Since nobody complained about it not working it's not worth fixing it
instead of deleting it (also I'm not sure it can be fixed with the
currently available APIs).
@mattam82
Copy link
Member

mattam82 commented Dec 4, 2024

If it's not broken don't fix it but if it's not used remove it!

@mattam82 mattam82 merged commit 34e50fe into MetaCoq:main Dec 4, 2024
5 checks passed
@SkySkimmer SkySkimmer deleted the rm-bugged-option branch December 4, 2024 15:31
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Jan 14, 2025
- use GADT to control the type of the value

- remove "Set ... Append" command, instead appending is a behaviour of
  the given option (currently Warnings and Debug)

- expose API for unsynchronized options (would allow to fix the code
  in MetaCoq/metacoq#1123 instead of removing it)
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.

2 participants