Skip to content

Optimize non-applicative stack nodes#20048

Closed
Tragicus wants to merge 2 commits intorocq-prover:masterfrom
Tragicus:casefix
Closed

Optimize non-applicative stack nodes#20048
Tragicus wants to merge 2 commits intorocq-prover:masterfrom
Tragicus:casefix

Conversation

@Tragicus
Copy link
Contributor

When unifying terms that both have a Case, Fix or Proj node in their arguments stack, Coq tries to reduce the non-applicative stack node, so we know that the node will never be reduced unless an evar is instantiated. Hence we unify and remove the longest suffix of the stack containing applicative nodes or the last node if none exists.

Fixes / closes #????

  • Added / updated test-suite.
  • Added changelog.
  • Added / updated documentation.
    • Documented any new / changed user messages.
    • Updated documented syntax by running make doc_gram_rsts.
  • Opened overlay pull requests.

When unifying terms that both have a Case, Fix or Proj node in their arguments stack, unify and remove the longest suffix of the stack containing applicative nodes or the last nodes if none exists.
@Tragicus Tragicus requested a review from a team as a code owner January 14, 2025 13:48
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jan 14, 2025
@Tragicus
Copy link
Contributor Author

This should solve the performance issue in #19987.

@SkySkimmer
Copy link
Contributor

@coqbot run full ci

@coqbot-app coqbot-app bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jan 14, 2025
@coqbot-app
Copy link
Contributor

coqbot-app bot commented Jan 14, 2025

🔴 CI failures at commit 03c0916 without any failure in the test-suite

✔️ Corresponding jobs for the base commit 818d1e0 succeeded

❔ Ask me to try to extract minimal test cases that can be added to the test-suite

🏃 @coqbot ci minimize will minimize the following targets: ci-fiat_crypto, ci-hott, ci-unimath
  • You can also pass me a specific list of targets to minimize as arguments.

@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jan 15, 2025
@gares gares added the request: full CI Use this label when you want your next push to trigger a full CI. label Jan 15, 2025
@SkySkimmer
Copy link
Contributor

@coqbot run full ci

@coqbot-app coqbot-app bot removed request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels Jan 23, 2025
@coqbot-app
Copy link
Contributor

coqbot-app bot commented Jan 23, 2025

🔴 CI failure at commit dcac3d4 without any failure in the test-suite

✔️ Corresponding job for the base commit 25659d3 succeeded

❔ Ask me to try to extract a minimal test case that can be added to the test-suite

🏃 @coqbot ci minimize will minimize the following target: ci-unimath
  • You can also pass me a specific list of targets to minimize as arguments.

@ppedrot ppedrot added the needs: fixing The proposed code change is broken. label Apr 29, 2025
@SkySkimmer
Copy link
Contributor

You can reopen if you get it to work, or if you can convince us that the failures are correct behaviour

@SkySkimmer SkySkimmer closed this May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: fixing The proposed code change is broken.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants