Skip to content

Conversation

rlepigre
Copy link
Contributor

@rlepigre rlepigre commented Nov 16, 2023

This PR is still a work in progress, but its aim is to implement the plan discussed with @SkySkimmer, @ppedrot, @herbelin, @mattam82 and @Janno on zulip and in a meeting.

The rough idea is to give projections their own transparency status via a new field in TransparentState.t, and to always keep the generated compatibility constants transparent (possibly, with an override flag to still be able to make them opaque).

Fixes / closes #18281

Depends #18373

Overlay PRs:

@rlepigre rlepigre requested review from a team as code owners November 16, 2023 15:09
@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 Nov 16, 2023
@rlepigre rlepigre marked this pull request as draft November 16, 2023 15:10
@rlepigre rlepigre changed the title Draft: Make projection constants transparent and allowing projections themselves to be made opaque Make projection constants transparent and allowing projections themselves to be made opaque Nov 16, 2023
@rlepigre
Copy link
Contributor Author

Another thing I'm wondering: in kernel/cClosure.ml, projection unfolding happens in knh, which it should morally happen in knr, right? Is it something that needs changing here?

@rlepigre rlepigre force-pushed the br/fix-18281 branch 3 times, most recently from ab04641 to 3691887 Compare November 23, 2023 10:53
Copy link
Member

@herbelin herbelin left a comment

Choose a reason for hiding this comment

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

To ease the reading, maybe some commits could go in a stand-alone PR (e.g. the renaming involving Evaluable).

@rlepigre
Copy link
Contributor Author

To ease the reading, maybe some commits could go in a stand-alone PR (e.g. the renaming involving Evaluable).

I thought of doing this, but then decided not to since this would delay this work quite a bit as overlays would likely be needed for both steps (rename, and then addition of a constructor for primitive projections).

Comment on lines 131 to 139
let make_flag_constant = function
| Evaluable.EvalVarRef id -> fVAR id
| Evaluable.EvalConstRef sp -> fCONST sp
| Evaluable.EvalProjectionRef p -> fPROJ p
| Evaluable.EvalVarRef id -> [fVAR id]
| Evaluable.EvalConstRef sp ->
begin
match Structures.PrimitiveProjections.find_opt sp with
| None -> [fCONST sp]
| Some p -> [fCONST sp; fPROJ p]
end
| Evaluable.EvalProjectionRef p -> [fPROJ p; fCONST (Projection.Repr.constant p)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure this is what we want, but @Janno and I can't think of an alternative. Basically, when you give delta flags, they somehow need to talk about both the projection and the compatibility constant. If it was just the projection, the compatibility constant is not unfolded (even though it should always be morally transparent).

@Janno
Copy link
Contributor

Janno commented Nov 28, 2023

@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 Nov 28, 2023
Copy link
Contributor

coqbot-app bot commented Nov 28, 2023

🔴 CI failures at commit 838cc25 without any failure in the test-suite

✔️ Corresponding jobs for the base commit b06166b 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-bedrock2, ci-hott, ci-itauto, ci-mathcomp_1, ci-perennial, ci-unimath
  • You can also pass me a specific list of targets to minimize as arguments.
  • If you tag me saying @coqbot ci minimize all, I will additionally minimize the following targets (which I do not suggest minimizing):
    • ci-cross_crypto (because base job at b06166b failed)
    • ci-fiat_parsers (because base job at b06166b failed)

@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 Nov 28, 2023
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 5, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 5, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 5, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 5, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 5, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 6, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 6, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 6, 2024
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 6, 2024
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 7, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 7, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 7, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 7, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 11, 2024
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 12, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq-elpi that referenced this pull request Mar 13, 2024
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 13, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 13, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 14, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 15, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 15, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq that referenced this pull request Mar 15, 2024
… allowing projections themselves to be made opaque

Reviewed-by: ppedrot
Ack-by: Janno
Ack-by: herbelin
Ack-by: jfehrle
Ack-by: SkySkimmer
Co-authored-by: ppedrot <ppedrot@users.noreply.github.com>
rlepigre pushed a commit to rlepigre/coq-serapi that referenced this pull request Mar 15, 2024
rlepigre pushed a commit to rlepigre/coq-serapi that referenced this pull request Mar 15, 2024
jellooo038 added a commit to impermeable/coq-waterproof that referenced this pull request Aug 20, 2024
* Changes for compatibility with 8.19

* Adapt files for the build process

* Update README.md

* Don't use opam to build and install for now

* Adapt dune files to make build with dune possible

* Change name field in dune file to Waterproof

* Revert to dune language 3.6

* Remove line on theory Ltac2

* Adapt to rocq-prover/rocq#17836 (sort poly)

* Revert "Adapt to rocq-prover/rocq#17836 (sort poly)"

* Adapt to rocq-prover/rocq#18174 (Clenv.unify takes cv_pb)

* Adapt to rocq-prover/rocq#17836 (sort poly) (#28)

* Adapt to rocq-prover/rocq#18280 (case relevance outside case info) (#37)

* Merge features of version 2.1.1 into coq-master (#46)

* [build] Initial port to Dune

This was done at the interest of Vincent WENDLING for jsCoq use.

The setup is fairly standard, other than excluding the deprecated
dir (which doesn't compile)

* Update README.md

* Update dune file in theories

* Change importing Ltac2 modules and build only with dune

* Update version numbers

* Restore changes file and rename license file

* Fix metadata in files

* Add template tags

* Add @install, minimal dune version, dev-repo to opam file

* Added infrastructure for enforcing users to mention use of definitions.

* Removed some unnecessary lemma's and corresponding tactics (that were apparently throwing errors anyway).

* Restored old lemmas in 'SupAndInf' necessary for keeping limsup file working. To be rewritten in future.

* Moved tests in SupAndInf.v file to separate test file. Small bug fix in rewrite rule sup and infimum.

* 'We need to show'-tactic now also accepts equivalent goals. Added 'By ...' clause to allow for an additional lemma from which equivalence follows.

* Replaced use of 'it suffices to show' by 'we need to show' for unfolding definitions.

* Added testcases new behaviour tactic.

* Improved feedback given new restricted proof automation.

* Updated documentation.

* Small change in error message.

* Moved 'AutomationFailure' exception type to 'Wateprove' file.

* Improved feedback 'ItHolds' for new restricted proof automation.

* Replaced custom error shielded goal by standard error that can be caught using Ltac2.

* Added 'Since ...' clause as alternative to 'By ...' that accepts statements instead of references. (No 'Since ... we need to show ...' because the 'By'-version is to be removed soon. )

* Removed acceptation equivalent goals from 'We need to show' tactic.

* Improved feedback 'By ... we conclude ...' for new restricted automated proof finding. Added 'Since ... we conclude that ...' alternative.

* Improved feedback, now it says which part of a chain of (in)equalities it failed to prove. Refactored checking whether additional lemma is actually used: now check is done in 'Waterprove.v', it throws errors with relevant information that other tactics use to display user-readable errors.

* 'Contradiction' now tries to find a contradiction to the previous statement.

* Moved 'Obtain' tactic to separate file.

* Simplified old notation 'Obtain' tactic. Now it automatically tries to destruct previous statement and users no longer need to specify the corresponding property.

* Simplified names for hypotheses not labeled by user.

* Added 'change' to 'Expand definition', so putting in different dummy variables actually has effect.

* Added StateGoal wrappers to subgoals of non-natural induction.

* Added workaround for unexpected anomalies in restricted automation.

* Strengthened workaround. Both to prevent more anomalies with hypotheses and to prevent endless searching for proof with hypothesis because it is used implicitly by the 'assumption' tactic.

* Replaced axiomatic definitions with locked ones. Also strengthened shielded automation to depth 3 to be able to use definition supremum..

* Fixed dune build.

* feat: add warnings

* refactor: change the names of the warning functions

* feat: add error function

* Add files to _CoqProject.in

* feat: use the warning in Conclusion.v

* feat: deal with Rabs Rmax Rmin more easily by destructing them

* Readded shortened 'Obtain accoriding to ...' tactic.

* Reworked shielding to use Shorten database type. Includes new tactics for explicitly unfolding definitions, optimized for speed automation (i.e. they explicitly fail to prevent large growth search tree).

* Improved expanding def for sup and inf.

* Implemented user-level error throwing in tactics.

* Fixed small error from merge.

* Disabled debug mode automation.

* Reverted back to old SupAndInf.v file.

* Replaced wrapping after 'Expand the definition of ...' by throwing errors suggesting user to replace the line by a tactic with a similar effect as unfolding the definitions. (Also slightly simplified notation base case natural induction.)

* Added tactic for unfolding that prints a message instead of throwing an errror. For internal use by Waterproof editor only.

* chore: bump version number

* fix: add internal unfold for general terms and tests for internal unfold

* Hint fixes (#30)

fix: change hint priorities

* Automation debug (#31)

* fix: change hint priorities

* fix: change hint priorities

* fix: fight line endings

* feat: add switch for debugging of the automation

* feat: Add description on how to enable debugging to readme

* feat: use N1 instead of N in definition convergence

* Improve either (#32)

* fix: definition of divergence to infinity and min infinity

* feat: add double_is_even to wp_integers rather than subsequences

* feat: let either work with uninformative or if the goal is a prop

* feat: also separate either code for prop for three statements. Refactor some decidability statements

* refactor: use new either code in alternative characterization of continuity in R

* feat: add alternative characterization continuity for natural numbers as subset of metric space of reals

* feat: some adaptations for the continuity exercises

* chore: update changelog and bump version number in opam file

* Tactics for using strong induction to define index sequence (#33)

* Added strong induction for defining index sequence. Warning: uses fixed letters 'n' and 'k' for index sequence and induction variable.

* Removed 'Check' and 'Print's.

* Removed old code.

* Show version number (#34)

* feat: Add tactic to show version number

* fix: Add version file to _CoqProject.in

* Allow testing against a folder with dune's runtest and set version number (#35)

* feat: add exercises as dune test

* feat: add python script for testing

* feat: abstract the testing scripts and deal with errors better

* fix: call the correct test script

* Set Help to use default automation system. (#36)

* Change required version number

* Try to fix coq requirement

* Fix for problems with strong induction for defining index sequence. (#38)

* Fix for problems with strong induction for defining subsequence.

* Updated formatting goal wrapper.

* fix: some small fixes to be compatible with dev

* fix: change order fold_right arguments in index sequence

* fix: Small changes to the sequences and subsequences files because autorewrite no longer seems to work as before

* chore: Change version number

* try to allow for dev version

* fix: try with version numbers

* fix: try to fix version numbers

* fix: Remove unnecessary import in Sequences.v

* feat: add logging sentence for wp_autorewrite

* feat: add logging sentence for wp_autorewrite (#43)

* feat: create option to print rewrite hints (#44)

* fix: Fix autorewrite (the env variable didn't come through properly)

* fix: Compatibility with compilers >= 4.09.0 (#45)

* fix: Exclude s390x architecture

* refactor: put *.install and *.diags in gitignore

---------

Co-authored-by: Emilio Jesus Gallego Arias
Co-authored-by: Jelle

* Adapt to rocq-prover/rocq#18327 (projection opacity) (#41)

* Adapt to rocq-prover/rocq#18529 (no Dyn.anonymous) (#47)

* Adapt to rocq-prover/rocq#18624 (Tac2ffi / Tac2val split) (#49)

* Adapt to rocq-prover/rocq#18546. (#48)

* Adapt to rocq-prover/rocq#18880 (#52)

* Adapt to rocq-prover/rocq#18938 (EConstr.ERelevance) (#53)

* Update README.md

Update installation instructions

* Testmaster (#54)

Implement a better way to do a case analyse on a type

Co-authored-by: DikieDick

* fix: Change 'Variable' to 'local Parameter'

* feat: add a test for awp_autorewrite from coq-master

* Refactor: incorporate some changes from 8.17 and update version numbers (#57)

* [build] Fix use of plugin aliases in findlib loading. (#58)

Note that both lines where loading the same plugin, but activating
different syntax rules; that's not really allowed as it leaves the
system in a partial state. If something like that is needed, true two
plugins are necessary (Which is not hard to support nowadays).

* feat: Postponing choices in the Choose tactic (#59)

* Merge 8.17 into main (#61)

Main change according to (PR #58): we only declare one plugin now.

* Renamed variables to prevent having  actual exercise solution in repo (#64)

* Specialize (#51)

* feat: first version of waterproof specialize tactic

* feat: Improve new specialize tactic, so it throws an error when a wrong variable is getting introduced.

* Avoid doubling hypothesis, allow for multiple binders, prevent matching on functions, rename hypothesis to user-specified one

---------

Co-authored-by: Jelle <jellewemmenhove@live.nl>

* feat: Postponing choices in the Choose tactic

* No longer rename variables in exists-statements

---------

Co-authored-by: Jelle <jellewemmenhove@live.nl>

---------

Co-authored-by: Gaëtan Gilbert <gaetan.gilbert@skyskimmer.net>
Co-authored-by: Rodolphe Lepigre <lepigre@mpi-sws.org>
Co-authored-by: Pierre Roux <pierre.roux@onera.fr>
Co-authored-by: Emilio Jesús Gallego Arias <e+git@x80.org>
Co-authored-by: Jelle <jellewemmenhove@live.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: redesign The same functionality is being re-implemented in a different way.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evarconv fails to unify primitive projection with compatibility constant in closed terms
6 participants