Skip to content

chore: squeeze more simps#11281

Closed
adomani wants to merge 17 commits intomasterfrom
adomani/squeeze_simps
Closed

chore: squeeze more simps#11281
adomani wants to merge 17 commits intomasterfrom
adomani/squeeze_simps

Conversation

@adomani
Copy link
Copy Markdown
Contributor

@adomani adomani commented Mar 10, 2024

Another batch of squeezed simps, converted mostly to simp only, but also simpa, tauto, aesop and simp? says.

Ref: #11246


Open in Gitpod

@adomani adomani force-pushed the adomani/squeeze_simps branch from 4994bfe to 609a770 Compare March 11, 2024 17:47
@adomani
Copy link
Copy Markdown
Contributor Author

adomani commented Mar 12, 2024

@jcommelin, thanks! Once I am home, I will undo these squeezing and I will add these proof to the unlinted ones.

My impression is that the category theory ones need some new idea. The others, almost make me wish that <;> was banned... I may write a linter to remove it! 🤣

@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 14, 2024
Copy link
Copy Markdown
Contributor

@joneugster joneugster left a comment

Choose a reason for hiding this comment

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

@adomani do you want to resolve all merge conflicts once more? Then I think we could see that we get this merged quickly.

(and maybe do a bench! once you've done that, just for good measure)

Comment on lines +1794 to +1803
induction' s using WSeq.recOn with a s s <;>
simp only [nil_append, cons_append, destruct_cons, destruct_pure, liftRelAux_inl_inl,
LiftRelO, true_and, think_append, destruct_think, Computation.destruct_think,
liftRelAux_inr_inr]
· induction' S using WSeq.recOn with s S S <;>
simp only [nil_append, join_nil, cons_append, join_cons, destruct_think,
Computation.destruct_think, think_append, append_assoc, liftRelAux_inr_inr, join_think]
· induction' SS using WSeq.recOn with S SS SS <;>
simp only [join_nil, destruct_nil, destruct_pure, map_nil, liftRelAux_inl_inl, LiftRelO,
join_cons, join_think, destruct_think, Computation.destruct_think, map_cons]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this one of the instances where avoiding non-terminal simp obfuscates the proof unreasonably?

@joneugster joneugster self-assigned this Jun 26, 2024
@joneugster joneugster added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels Jun 26, 2024
@github-actions
Copy link
Copy Markdown

PR summary 8d36235876

Import changes

No significant changes to the import graph


Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/no_lost_declarations.sh short <optional_commit>

## more verbose report:
./scripts/no_lost_declarations.sh <optional_commit>

@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 26, 2024
@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jul 1, 2024
@joneugster
Copy link
Copy Markdown
Contributor

@adomani : I feel like this PR has been lying around for too long. The squeezes here either already have been fixed or should be addressed by your "flexible linter".

I think it's probably easier to close this PR and redo the things again on a fresh master branch if still relevant.

Please reopen the PR if you disagree.

@adomani
Copy link
Copy Markdown
Contributor Author

adomani commented Sep 11, 2024

I'm happy with closing this. As you probably know, I am no longer sure that squeezing simps is the correct answer to maintainability -- certainly not in all cases!

@YaelDillies YaelDillies deleted the adomani/squeeze_simps branch August 15, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-author A reviewer has asked the author a question or requested changes. merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants