Skip to content

[Merged by Bors] - feat: add Type*/Sort* and tests#6474

Closed
mattrobball wants to merge 1 commit intomasterfrom
mrb/mini_universes
Closed

[Merged by Bors] - feat: add Type*/Sort* and tests#6474
mattrobball wants to merge 1 commit intomasterfrom
mrb/mini_universes

Conversation

@mattrobball
Copy link
Copy Markdown
Contributor

@mattrobball mattrobball commented Aug 9, 2023

This adds Kyle Miller's elaborators for Type* and Sort* which provide implicit universe variables.

It also includes a few tests.

Co-authored-by: Kyle Miller kmill31415@gmail.com


Open in Gitpod

Copy link
Copy Markdown
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add a co-authored-by line for Kyle?

@semorrison, do you have any opinions on how the tests should be written?

@eric-wieser eric-wieser added the t-meta Tactics, attributes or user commands label Aug 9, 2023
@mattrobball
Copy link
Copy Markdown
Contributor Author

Thanks for catching that

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Aug 9, 2023

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Aug 9, 2023
bors bot pushed a commit that referenced this pull request Aug 9, 2023
This adds Kyle Miller's elaborators for `Type*` and `Sort*` which provide implicit universe variables.

It also includes a few tests.

Co-authored-by: Kyle Miller <kmill31415@gmail.com>
@bors
Copy link
Copy Markdown

bors bot commented Aug 10, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title feat: add Type*/Sort* and tests [Merged by Bors] - feat: add Type*/Sort* and tests Aug 10, 2023
@bors bors bot closed this Aug 10, 2023
@bors bors bot deleted the mrb/mini_universes branch August 10, 2023 00:40
kim-em pushed a commit that referenced this pull request Aug 14, 2023
This adds Kyle Miller's elaborators for `Type*` and `Sort*` which provide implicit universe variables.

It also includes a few tests.

Co-authored-by: Kyle Miller <kmill31415@gmail.com>
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
bors bot pushed a commit that referenced this pull request Aug 14, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
kim-em pushed a commit that referenced this pull request Aug 15, 2023
This adds Kyle Miller's elaborators for `Type*` and `Sort*` which provide implicit universe variables.

It also includes a few tests.

Co-authored-by: Kyle Miller <kmill31415@gmail.com>
kim-em pushed a commit that referenced this pull request Aug 15, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
kim-em pushed a commit that referenced this pull request Aug 17, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
kim-em pushed a commit that referenced this pull request Aug 17, 2023
Autoimplicits are [highly controversial](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/.60autoImplicit.20true.60.20is.20evil/near/355142480) and also [defeat the performance-improving work](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/autoimplicit.20and.20Type*/near/383878932) in #6474.

The intent of this PR is to make `autoImplicit` opt-in on a per-file basis, by disabling it in the lakefile and enabling it again with `set_option autoImplicit true` in the few files that rely on it.

That also keeps this PR small, as opposed to attempting to "fix" files to not need it any more.

I claim that many of the uses of `autoImplicit` in these files are accidental; situations such as:

* Assuming `variables` are in scope, but pasting the lemma in the wrong section
* Pasting in a lemma from a scratch file without checking to see if the variable names are consistent with the rest of the file
* Making a copy-paste error between lemmas and forgetting to add an explicit arguments.

Having `set_option autoImplicit false` as the default prevents these types of mistake being made in the 90% of files where `autoImplicit`s are not used at all, and causes them to be caught by CI during review.

I think there were various points during the port where we encouraged porters to delete the `universes u v` lines; I think having autoparams for universe variables only would cover a lot of the cases we actually use them, while avoiding any real shortcomings.

[A Zulip poll](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/mathlib4.3A.20autoImplicit.20false.20by.20default.3F/near/384265143) (after combining overlapping votes accordingly) was in favor of this change with `5:5:18` as the `no:dontcare:yes` vote ratio.

While this PR was being reviewed, a handful of files gained some more likely-accidental autoImplicits. In these places, `set_option autoImplicit true` has been placed locally within a section, rather than at the top of the file.
bors bot pushed a commit that referenced this pull request Sep 6, 2023
We clean up heart beat bumps after #6474.
bors bot pushed a commit that referenced this pull request Sep 6, 2023
We clean up heart beat bumps after #6474.
ebab pushed a commit that referenced this pull request Sep 11, 2023
We clean up heart beat bumps after #6474.
kodyvajjha pushed a commit that referenced this pull request Sep 22, 2023
We clean up heart beat bumps after #6474.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants