Skip to content

[Merged by Bors] - chore: Restore most of the mono attribute#2491

Closed
casavaca wants to merge 8 commits intomasterfrom
restore-mono
Closed

[Merged by Bors] - chore: Restore most of the mono attribute#2491
casavaca wants to merge 8 commits intomasterfrom
restore-mono

Conversation

@casavaca
Copy link
Copy Markdown
Contributor

@casavaca casavaca commented Feb 25, 2023

Restore most of the mono attribute now that #1740 is merged.

I think I got all of the monos.


Restore mono so that when porting, mono tactic has a better chance to just work.

Open in Gitpod

@casavaca casavaca added t-meta Tactics, attributes or user commands awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. awaiting-review and removed awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Feb 25, 2023
@mcdoll mcdoll added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Feb 26, 2023
@casavaca
Copy link
Copy Markdown
Contributor Author

  • Got syntax error on @[to_additive nsmul_le_nsmul_of_le_right (attr := mono)],
  • Got unknown attribute [sum_mono] on @[to_additive (attr := sum_mono, mono)]

I made some other changes at these 2 places (I don't know if that's a good change or not)...

@casavaca casavaca added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Feb 26, 2023
@mcdoll
Copy link
Copy Markdown
Member

mcdoll commented Feb 27, 2023

Oh, so the syntax is probably then @[to_additive (attr := mono) foo] and the second one should be @[to_additive (attr := mono) sum_mono], sum_mono is the name of the lemma not an attribute (I was being sloppy, sorry).

@casavaca casavaca requested review from jcommelin and mcdoll February 27, 2023 20:38
Copy link
Copy Markdown
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

I think you don't need to manually add the mono attribute to additive versions if you pass mono via attr := to to_additive.

@fpvandoorn can you confirm this?


-- Porting note: commented out the next line since tactic `mono` does not yet exist
-- attribute [mono] sum_mono
attribute [mono] sum_mono
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I conjecture that this is not needed if you have the attr := mono on L680.


-- Porting note: removed `mono` attribute, not implemented yet.
-- attribute [mono] nsmul_le_nsmul_of_le_right
attribute [mono] nsmul_le_nsmul_of_le_right
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem.

#align set.mul_subset_mul Set.mul_subset_mul
#align set.add_subset_add Set.add_subset_add

attribute [mono] add_subset_add
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem.

@fpvandoorn
Copy link
Copy Markdown
Member

@[to_additive (attr := mono)] indeed adds the mono attribute to both the additive and multiplicative declaration.

@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Feb 28, 2023
@casavaca casavaca added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Feb 28, 2023
Copy link
Copy Markdown
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@kim-em kim-em added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Mar 1, 2023
bors bot pushed a commit that referenced this pull request Mar 1, 2023
Restore most of the `mono` attribute now that #1740 is merged.

I think I got all of the `mono`s.
@bors
Copy link
Copy Markdown

bors bot commented Mar 1, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore: Restore most of the mono attribute [Merged by Bors] - chore: Restore most of the mono attribute Mar 1, 2023
@bors bors bot closed this Mar 1, 2023
@bors bors bot deleted the restore-mono branch March 1, 2023 08:32
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.

5 participants