Skip to content

feat: Qq helpers to extract literals#21991

Closed
YaelDillies wants to merge 1 commit intomasterfrom
qq_nat_lit
Closed

feat: Qq helpers to extract literals#21991
YaelDillies wants to merge 1 commit intomasterfrom
qq_nat_lit

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor


Open in Gitpod

@YaelDillies YaelDillies added the t-meta Tactics, attributes or user commands label Feb 17, 2025
@github-actions
Copy link
Copy Markdown

PR summary e2a5eb5f68

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Util.Qq 21 2 -19 (-90.48%)
Import changes for all files
Files Import difference
There are 3056 files with changed transitive imports taking up over 135469 characters: this is too many to display!
You can run scripts/import_trans_difference.sh all locally to see the whole output.

Declarations diff

+ intLitQq?
+ natLitQq?
+ rawIntLit!
+ rawIntLit?
+ rawNatLit!
+ rawNatLit!_lit_natVal

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

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

The doc-module for script/declarations_diff.sh contains some details about this script.


No changes to technical debt.

You can run this locally as

./scripts/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).


See `Qq.natLitQq?` for a Qq version of this that also handles non-raw literals.

Note this is equivalent to `Lean.Expr.natLit!`, which is misnamed. -/
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser Feb 17, 2025

Choose a reason for hiding this comment

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

Can you just rename this? It's in Batteries not core.

Comment on lines +19 to +20
/-- Returns the integer from a raw integer literal expression, or `none` if the expression isn't
a raw integer literal.
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 claim this is not something you ever need to do. "Raw integer literal expression" is a concept from only within norm_num, where you get handed the cases separately anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"this" = "using this function"? Then what should I replace it with in the Qq function?

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.

What do you need the Qq function for?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Paul wanted it to extract10 from Int.divisorsAntidiag 10

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.

There you don't have a "raw integer literal" anyway, you can match on OfNat.ofNat (nat_lit n) and -OfNat.ofNat (nat_lit n)

Comment on lines +43 to +45
def natLitQq? : Q(Nat) → MetaM (Option Nat)
| ~q(OfNat.ofNat $n) => pure n.rawNatLit?
| e => pure e.rawNatLit?
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.

While this seems like a nice enough thing to have, we already have a baffling number of these, and I don't see why your use case needs your new one when previous ones didn't.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Uh that's terrible. They all look misnamed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you agree that it would be worth unifying all these?

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 think they all do various amounts of post-processing (raw / non-raw, numeral / succ / zero, etc).

For the purpose of the first version your simproc, I think it's fine to support only positive numerals for now, where you can just copy the approach I used for the prime factors simproc.

@eric-wieser eric-wieser added the awaiting-author A reviewer has asked the author a question or requested changes. label Feb 18, 2025
@YaelDillies
Copy link
Copy Markdown
Contributor Author

Looks like I don't need this anymore

@YaelDillies YaelDillies deleted the qq_nat_lit branch February 18, 2025 12:22
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. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants