Skip to content

[Merged by Bors] - feat: to_additive raises linter errors; nested to_additive#1819

Closed
fpvandoorn wants to merge 7 commits intomasterfrom
toadditivelint
Closed

[Merged by Bors] - feat: to_additive raises linter errors; nested to_additive#1819
fpvandoorn wants to merge 7 commits intomasterfrom
toadditivelint

Conversation

@fpvandoorn
Copy link
Copy Markdown
Member

@fpvandoorn fpvandoorn commented Jan 24, 2023

  • Turn info messages of to_additive into linter errors
  • Allow @[to_additive (attr := to_additive)] to additivize the generated lemma. This is useful for Pow -> SMul -> VAdd lemmas. We can write e.g. @[to_additive (attr := to_additive, simp)] to add the simp attribute to all 3 generated lemmas, and we can provide other options to each to_additive call separately (specifying a name / reorder).
  • The previous point was needed to cleanly get rid of some linter warnings. It also required some additional changes (addToAdditiveAttr now returns a value, turn a few (meta) definitions into mutual partial def, reorder some definitions, generalize additivizeLemmas to lists of more than 2 elements) that should have no visible effects for the user.

Open in Gitpod

@fpvandoorn fpvandoorn added the awaiting-author A reviewer has asked the author a question or requested changes. label Jan 24, 2023
@kim-em kim-em added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Jan 24, 2023
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Jan 24, 2023

This PR/issue depends on:

@fpvandoorn fpvandoorn added awaiting-review t-meta Tactics, attributes or user commands awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. and removed awaiting-author A reviewer has asked the author a question or requested changes. awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Jan 24, 2023
@fpvandoorn fpvandoorn changed the title feat: to_additive improvements feat: to_additive raises linter errors; nested to_additive Jan 25, 2023
@fpvandoorn
Copy link
Copy Markdown
Member Author

fpvandoorn commented Jan 27, 2023

note: please delegate - not merge - this PR. It likely raises errors when merging master

@kim-em kim-em added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jan 29, 2023
Copy link
Copy Markdown
Member

@gebner gebner 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+

I'm not sure I'm a fan of using logLinter in functions that are not linters.

@bors
Copy link
Copy Markdown

bors bot commented Feb 16, 2023

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

@kim-em kim-em added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Feb 16, 2023
@fpvandoorn
Copy link
Copy Markdown
Member Author

bors d+

I'm not sure I'm a fan of using logLinter in functions that are not linters.

Thanks for the delegation.

I'm happy to use something else. This specifically was requested by @digama0 here.

@gebner
Copy link
Copy Markdown
Member

gebner commented Feb 17, 2023

I'm happy to use something else. This specifically was requested by @digama0 here.

@digama0 Do you plan to make all warnings disableable?

@fpvandoorn
Copy link
Copy Markdown
Member Author

bors merge

I'm happy to change this to something else in a future PR, if we reach a consensus.

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Feb 17, 2023
bors bot pushed a commit that referenced this pull request Feb 17, 2023
* Turn info messages of `to_additive` into linter errors
* Allow `@[to_additive (attr := to_additive)]` to additivize the generated lemma. This is useful for `Pow -> SMul -> VAdd` lemmas. We can write e.g. `@[to_additive (attr := to_additive, simp)]` to add the `simp` attribute to all 3 generated lemmas, and we can provide other options to each `to_additive` call separately (specifying a name / reorder).
* The previous point was needed to cleanly get rid of some linter warnings. It also required some additional changes (`addToAdditiveAttr` now returns a value, turn a few (meta) definitions into `mutual partial def`, reorder some definitions, generalize `additivizeLemmas` to lists of more than 2 elements) that should have no visible effects for the user.
@bors
Copy link
Copy Markdown

bors bot commented Feb 17, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: to_additive raises linter errors; nested to_additive [Merged by Bors] - feat: to_additive raises linter errors; nested to_additive Feb 17, 2023
@bors bors bot closed this Feb 17, 2023
@bors bors bot deleted the toadditivelint branch February 17, 2023 17:44
mo271 pushed a commit that referenced this pull request Feb 18, 2023
* Turn info messages of `to_additive` into linter errors
* Allow `@[to_additive (attr := to_additive)]` to additivize the generated lemma. This is useful for `Pow -> SMul -> VAdd` lemmas. We can write e.g. `@[to_additive (attr := to_additive, simp)]` to add the `simp` attribute to all 3 generated lemmas, and we can provide other options to each `to_additive` call separately (specifying a name / reorder).
* The previous point was needed to cleanly get rid of some linter warnings. It also required some additional changes (`addToAdditiveAttr` now returns a value, turn a few (meta) definitions into `mutual partial def`, reorder some definitions, generalize `additivizeLemmas` to lists of more than 2 elements) that should have no visible effects for the user.
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). 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