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

Stop using auto with * in intuition #496

Closed
wants to merge 2 commits into from

Conversation

SkySkimmer
Copy link
Contributor

No description provided.

Copy link
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

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

Thanks for this pull request. I saw the warnings and I know intuition is about to change. Earlier this year, I tried to do fix each call to intuition but didn't go all the way to the end. Your code is nicer because you just change the Tauto.intuition_solver tactic, not each use of intuition. However, Tauto.intuition_solver is not supported in Coq 8.14 and 8.12 (the oldest version supported by CompCert), so I'll probably go back to my approach, with the additional knowledge gained from your code. (I didn't know about the exfalso hint database...)

Also: you left an intuition info_auto with * , I took the liberty to replace it by intuition and it works just fine.

@SkySkimmer
Copy link
Contributor Author

However, Tauto.intuition_solver is not supported in Coq 8.14 and 8.12 (the oldest version supported by CompCert),

Tactic Notation "intuition" := (intuition some_solver) also works as long as intuition isn't called through another tactic

@SkySkimmer
Copy link
Contributor Author

intuition is about to change

There is no rush I think. The warning is enough to avoid inadvertently relying on it in new code, and there are too many users of default intuition in our CI for us to bother porting without a stronger motivation IMO.

xavierleroy added a commit that referenced this pull request Jul 10, 2023
Coq 8.17 warns about this, since at some point in the future
`intuition` will become `intuition auto`.

This commit rewrites a number of uses of `intuition` to avoid relying on
the implicit `intuition auto with *`.

Co-authored-by: Gaëtan Gilbert <gaetan.gilbert@skyskimmer.net>
@xavierleroy
Copy link
Contributor

Using this PR as a guide, I was able to silence all the 8.17 warnings about intuition, while remaining compatible with 8.12 and up. See 974fbd8. You're listed as co-author on this commit. Now we can close this PR. Thanks again.

@SkySkimmer SkySkimmer deleted the intuit-auto branch July 10, 2023 12:12
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