Skip to content

feat: do not export theorem bodies#8090

Merged
Kha merged 9 commits intoleanprover:masterfrom
Kha:push-qqqmpwywyywr
Apr 25, 2025
Merged

feat: do not export theorem bodies#8090
Kha merged 9 commits intoleanprover:masterfrom
Kha:push-qqqmpwywyywr

Conversation

@Kha
Copy link
Copy Markdown
Member

@Kha Kha commented Apr 24, 2025

This PR adjusts the experimental module system to elide theorem bodies (i.e. proofs) from being imported into other modules.

@Kha Kha added the changelog-language Language features and metaprograms label Apr 24, 2025
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Apr 24, 2025
@ghost
Copy link
Copy Markdown

ghost commented Apr 24, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 66c00d33d47b75171f9261e26c602b326202b67a --onto 02f7a1dd41f034939acf49a5c2c590076cb26d8e. You can force Mathlib CI using the force-mathlib-ci label. (2025-04-24 14:38:31)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 68d9d14d44ec95afe26968fff7a682968b2d2fb9 --onto 02f7a1dd41f034939acf49a5c2c590076cb26d8e. You can force Mathlib CI using the force-mathlib-ci label. (2025-04-25 09:44:35)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 8195f7050283953d361e221d74c7607f6396b0d7 --onto 416e07a68e5d9b884de8af4836497a7cc2414c8f. You can force Mathlib CI using the force-mathlib-ci label. (2025-04-25 17:04:25)

@Kha Kha force-pushed the push-qqqmpwywyywr branch from 617d644 to 008d4c7 Compare April 24, 2025 17:03
@Kha Kha requested review from leodemoura and mhuisi as code owners April 24, 2025 17:03
@Kha Kha added the merge-ci Enable merge queue CI checks for PR. In particular, produce artifacts for all major platforms. label Apr 24, 2025
@Kha
Copy link
Copy Markdown
Member Author

Kha commented Apr 24, 2025

!bench

@Kha Kha force-pushed the push-qqqmpwywyywr branch from 008d4c7 to 54994f3 Compare April 24, 2025 17:17
@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 008d4c7.
The entire run failed.
Found no significant differences.

@Kha Kha requested a review from hargoniX as a code owner April 24, 2025 17:24
@Kha
Copy link
Copy Markdown
Member Author

Kha commented Apr 24, 2025

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 5a123dc.
The entire run failed.
Found no significant differences.

@Rob23oba
Copy link
Copy Markdown
Contributor

Rob23oba commented Apr 24, 2025

I imagine that changing theorems to axioms will cause quite a few problems -- an alternative I see that might be less intrusive is to replace the body with some dummy value like _proof_omitted. The advantage is obviously that we need to refactor less parts of code to treat the case of theorems without an imported body. The disadvantage is obviously that the resulting term is not type-correct, an alternative is to use lcProof so at least it's type-correct except it's unsafe within a safe declaration. Then it would at least be easy to detect (TheoremInfo.isProofOmitted := value == lcProof very roughly). Not sure what works best, maybe we'd want to add a new unsafe axiom particularly for this case even..?

@Rob23oba
Copy link
Copy Markdown
Contributor

Rob23oba commented Apr 24, 2025

Also I guess this could get weird w.r.t. #print axioms... If theorems become axioms then suddenly everything is an axiom, if theorems stay theorems but have their body change to a dummy value then suddenly sorry proofs depend on no axioms..? One really bad hack would be to instead replace the bodies with proofOmitted @Classical.choice @Quot.sound @propext depending on which axioms are used but I'm not sure what to think of that...

@Kha Kha force-pushed the push-qqqmpwywyywr branch 2 times, most recently from 4feae53 to dbf50e6 Compare April 24, 2025 18:21
@Kha
Copy link
Copy Markdown
Member Author

Kha commented Apr 24, 2025

Thanks for thinking through this!

I imagine that changing theorems to axioms will cause quite a few problems

Do you have some specific concerns in mind? Current evidence suggests the opposite, the transitive uses of thmInfo patterns are surprisingly few.

Also I guess this could get weird w.r.t. #print axioms

Look forward to a follow-up PR introducing an environment extension for this. This was the only non-trivial thmInfo use case I found in core.

@Rob23oba
Copy link
Copy Markdown
Contributor

Hmm I guess I can't really say much about where thmInfo is used either but the hack in MutualDef is definitely not nice (in particular for matching on syntax). I'm not 100% sold on @[dsimp] (well that is, depending on how it would work). Just a @[dsimp] tag instead of @[simp] would definitely not work well because

  1. what about other simp attributes?
  2. how about non-simp theorems that are still rfl and used in simp calls? (_def lemmas come to mind)

One way that you could address these would definitely be to make @[dsimp] an additional tag (so @[simp, dsimp]) and the @[dsimp] extension is just a set of those theorems without structure (so that when you add a lemma to simp, it can check whether it is in this set).
That would work but it requires the foresight to tag all defeq lemmas that you might want to use in dsimp with @[dsimp] so not great either.

@Kha Kha force-pushed the push-qqqmpwywyywr branch from dbf50e6 to 205d08d Compare April 24, 2025 21:00
@Kha
Copy link
Copy Markdown
Member Author

Kha commented Apr 24, 2025

@nomeata will work on the [dsimp] part separately

@Kha
Copy link
Copy Markdown
Member Author

Kha commented Apr 24, 2025

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 205d08d.
There were significant changes against commit 68d9d14:

  Benchmark                      Metric                       Change
  =============================================================================
+ Init size                      bytes .olean                 -55.1%
- Init size                      bytes .olean.private      283914.3%
- Init.Data.BitVec.Lemmas        branch-misses                 64.3%  (268.9 σ)
- Init.Data.BitVec.Lemmas        branches                      12.8%  (227.4 σ)
- Init.Data.BitVec.Lemmas        instructions                  14.4%  (227.3 σ)
- Init.Data.BitVec.Lemmas        maxrss                        53.6%   (23.0 σ)
- Init.Data.BitVec.Lemmas        task-clock                    87.9% (1561.4 σ)
+ Init.Data.BitVec.Lemmas        wall-clock                   -25.7% (-506.0 σ)
+ Init.Data.List.Sublist async   branches                      -4.5%  (-38.4 σ)
+ Init.Data.List.Sublist async   instructions                  -5.0%  (-37.4 σ)
+ Init.Data.List.Sublist async   task-clock                   -20.5%  (-26.2 σ)
- Init.Data.List.Sublist async   wall-clock                    15.0%   (12.9 σ)
+ bv_decide_large_aig.lean       task-clock                    -5.6%  (-19.5 σ)
+ bv_decide_large_aig.lean       wall-clock                    -5.5%  (-23.3 σ)
- identifier auto-completion     branches                       3.1%  (905.5 σ)
- identifier auto-completion     instructions                   2.7% (1147.7 σ)
+ import Lean                    task-clock                    -3.8%  (-24.0 σ)
+ import Lean                    wall-clock                    -3.7%  (-31.6 σ)
+ parser                         task-clock                    -2.3%  (-10.4 σ)
+ parser                         wall-clock                    -2.3%  (-10.3 σ)
- qsort                          task-clock                     2.3%   (15.9 σ)
- qsort                          wall-clock                     2.3%   (15.8 σ)
- reduceMatch                    task-clock                     1.9%   (11.5 σ)
+ stdlib                         blocked                      -21.8%  (-69.2 σ)
- stdlib                         dsimp                          5.6%  (279.7 σ)
- stdlib                         maxrss                         9.9%   (11.1 σ)
- stdlib                         process pre-definitions        7.0%   (18.7 σ)
- stdlib                         tactic execution              13.0%  (209.5 σ)
- stdlib                         task-clock                     5.1%  (108.9 σ)
- stdlib                         type checking                  4.1%   (65.5 σ)
+ stdlib size                    bytes .olean                  -9.8%
- stdlib size                    bytes .olean.private      283914.3%

@Kha Kha force-pushed the push-qqqmpwywyywr branch from 205d08d to ad2e623 Compare April 25, 2025 07:35
Comment on lines +574 to +575
-- For theorems, `isProp` has already been checked at declaration time
unless wasOriginallyTheorem (← getEnv) declName do
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.

@leodemoura FYI - I'm assuming this was the intention of the check and that it is still helpful even if it requires an additional environment extension lookup. Also please let me know if you can think of a better name for the function!

@Kha Kha force-pushed the push-qqqmpwywyywr branch 2 times, most recently from 4eed60d to 6702c30 Compare April 25, 2025 07:48
@Kha Kha added release-ci Enable all CI checks for a PR, like is done for releases and removed merge-ci Enable merge queue CI checks for PR. In particular, produce artifacts for all major platforms. labels Apr 25, 2025
@Kha Kha enabled auto-merge April 25, 2025 07:56
@Kha Kha disabled auto-merge April 25, 2025 08:36
@Kha Kha force-pushed the push-qqqmpwywyywr branch from 6702c30 to 21e4b56 Compare April 25, 2025 15:33
@Kha Kha force-pushed the push-qqqmpwywyywr branch from 21e4b56 to 35d0124 Compare April 25, 2025 20:05
@Kha Kha removed the release-ci Enable all CI checks for a PR, like is done for releases label Apr 25, 2025
@Kha Kha enabled auto-merge April 25, 2025 20:06
@Kha Kha added this pull request to the merge queue Apr 25, 2025
Merged via the queue into leanprover:master with commit 62c6edf Apr 25, 2025
18 checks passed
@Kha Kha deleted the push-qqqmpwywyywr branch April 26, 2025 07:06
github-merge-queue bot pushed a commit that referenced this pull request Apr 26, 2025
#8090 accidentally affected `dsimp` applications even outside the module
system, restore previous extension data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-language Language features and metaprograms toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants