Skip to content

[Merged by Bors] - chore: fix tests after #6528 disabled autoimplicits#6597

Closed
alexjbest wants to merge 2 commits intomasterfrom
alexjbest/autoimplicittests
Closed

[Merged by Bors] - chore: fix tests after #6528 disabled autoimplicits#6597
alexjbest wants to merge 2 commits intomasterfrom
alexjbest/autoimplicittests

Conversation

@alexjbest
Copy link
Copy Markdown
Member

@alexjbest alexjbest commented Aug 15, 2023

These work on the command line and therefore didn't fail in CI (as lake env lean blah.lean doesn't pick up the extra lean args?) but as soon as you open one of these files in vscode you get a sea of red errors.

I tried to fix a couple in a minimal way, if there was just for example one universe level missing, for most files the "fix" for now is just reenabling autoimplicits for the file.
This revealed a couple of tests that were incorrectly using an implicit type when they thought they were using Nat.
Two files had issues, that we may want to follow up on test/superscript.lean which failed even with autoimplicits turned on when using an autoimplicit superscript variable (unsure if this was intentional).
And the ToExpr deriving handler seems to rely on autoimplicits somehow to work properly in the presence of universe, if an explicit universe variable is added there is still an error.


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.

bors d+

Thanks!

@bors
Copy link
Copy Markdown

bors bot commented Aug 15, 2023

✌️ alexjbest can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@ghost ghost added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Aug 15, 2023
@eric-wieser
Copy link
Copy Markdown
Member

eric-wieser commented Aug 15, 2023

Two files had issues, that we may want to follow up on test/superscript.lean which failed even with autoimplicits turned on when using an autoimplicit superscript variable (unsure if this was intentional). And the ToExpr deriving handler seems to rely on autoimplicits somehow to work properly in the presence of universe, if an explicit universe variable is added there is still an error.

Can you open an issue for each of these two, and reference it in a TODO comment?

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@alexjbest
Copy link
Copy Markdown
Member Author

I was planning to look into them/discuss on zulip myself before reporting. I think that should be enough, we can always add the comments later if it does turn out they should become issues

@alexjbest
Copy link
Copy Markdown
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Aug 15, 2023
These work on the command line and therefore didn't fail in CI (as `lake env lean blah.lean` doesn't pick up the extra lean args?) but as soon as you open one of these files in vscode you get a sea of red errors.

I tried to fix a couple in a minimal way, if there was just for example one universe level missing, for most files the "fix" for now is just reenabling autoimplicits for the file.
This revealed a couple of tests that were incorrectly using an implicit type when they thought they were using Nat.
Two files had issues, that we may want to follow up on  `test/superscript.lean` which failed even with autoimplicits turned on when using an autoimplicit superscript variable (unsure if this was intentional).
And the `ToExpr` deriving handler seems to rely on autoimplicits somehow to work properly in the presence of universe, if an explicit universe variable is added there is still an error.
@bors
Copy link
Copy Markdown

bors bot commented Aug 15, 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 chore: fix tests after #6528 disabled autoimplicits [Merged by Bors] - chore: fix tests after #6528 disabled autoimplicits Aug 15, 2023
@bors bors bot closed this Aug 15, 2023
@bors bors bot deleted the alexjbest/autoimplicittests branch August 15, 2023 23:33
kim-em pushed a commit that referenced this pull request Aug 17, 2023
These work on the command line and therefore didn't fail in CI (as `lake env lean blah.lean` doesn't pick up the extra lean args?) but as soon as you open one of these files in vscode you get a sea of red errors.

I tried to fix a couple in a minimal way, if there was just for example one universe level missing, for most files the "fix" for now is just reenabling autoimplicits for the file.
This revealed a couple of tests that were incorrectly using an implicit type when they thought they were using Nat.
Two files had issues, that we may want to follow up on  `test/superscript.lean` which failed even with autoimplicits turned on when using an autoimplicit superscript variable (unsure if this was intentional).
And the `ToExpr` deriving handler seems to rely on autoimplicits somehow to work properly in the presence of universe, if an explicit universe variable is added there is still an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants