Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[primitive floats] Deprecate frexp and ldexp #15085

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

proux01
Copy link
Contributor

@proux01 proux01 commented Oct 28, 2021

Now that we have signed primitive integers, we would like to use them
in frexp and ldexp but the name was taken by functions with exponent
in Z. These functions are renamed Z.frexp and Z.ldexp so that frexp and
ldexp could later be added with signed primitive integer exponent.

  • Added changelog.

@proux01 proux01 requested a review from a team as a code owner October 28, 2021 14:38
@proux01
Copy link
Contributor Author

proux01 commented Oct 28, 2021

Cc @erikmd and @silene who may have an opinion.

@silene
Copy link
Contributor

silene commented Oct 28, 2021

I am confused. I thought frexp and ldexp were using biased primitive integers. It was not as good as signed primitive integers, but certainly not as bad as Z numbers.

@proux01
Copy link
Contributor Author

proux01 commented Oct 28, 2021

Those are called frshiftexp and ldshiftexp.

@erikmd
Copy link
Member

erikmd commented Oct 28, 2021

Thanks @proux01 for opening this.
I agree with the fact that it'll be nice to put forth the primitive signed integers over mere Z integers!

However, I am wondering whether one such renaming could be done in a more "modular" way (like the BinInt theory)?

I mean, renaming FloatOps.frexp to something like FloatOps.Z.frexp

@proux01
Copy link
Contributor Author

proux01 commented Oct 29, 2021

Indeed, done.

Copy link
Member

@erikmd erikmd left a comment

Choose a reason for hiding this comment

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

Thanks @proux01, this looks fine (except two lines that needs an update)

theories/Floats/FloatLemmas.v Outdated Show resolved Hide resolved
theories/Floats/FloatLemmas.v Outdated Show resolved Hide resolved
@erikmd
Copy link
Member

erikmd commented Oct 30, 2021

Thanks, it seems the changelog is the last missing part

@proux01 proux01 added part: primitive types Primitive ints, floats, arrays, etc. part: standard library The standard library stdlib. kind: cleanup Code removal, deprecation, refactorings, etc. labels Nov 2, 2021
@proux01 proux01 added this to the 8.15+rc1 milestone Nov 2, 2021
Now that we have signed primitive integers, we would like to use them
in frexp and ldexp but the name was taken by functions with exponent
in Z. These functions are renamed frZexp and ldZexp so that frexp and
ldexp could later be added with signed primitive integer exponent.

Co-authored-by: Erik Martin-Dorel <erik.martin-dorel@irit.fr>
@proux01
Copy link
Contributor Author

proux01 commented Nov 2, 2021

@erikmd done, would you be the assignee?

@erikmd erikmd self-assigned this Nov 2, 2021
@erikmd
Copy link
Member

erikmd commented Nov 2, 2021

Sure. If no one objects, I propose to merge this cleanup PR on tomorrow Wednesday evening.

@erikmd
Copy link
Member

erikmd commented Nov 3, 2021

@coqbot merge now

@coqbot-app
Copy link
Contributor

coqbot-app bot commented Nov 3, 2021

@erikmd: You can't merge this PR because you're not a member of the @coq/pushers team.

@erikmd
Copy link
Member

erikmd commented Nov 3, 2021

@silene can you be the assignee then?

@erikmd erikmd removed their assignment Nov 3, 2021
@ejgallego
Copy link
Member

Oh, let us fix this @erikmd

@ejgallego
Copy link
Member

@erikmd not sure what's the best team for "primitive floats maintainers", I've added you to the vm-native-maintainers for now, but let's refine this if needed; please try again.

@ejgallego
Copy link
Member

And sorry for the annoyance.

@erikmd
Copy link
Member

erikmd commented Nov 3, 2021

@erikmd not sure what's the best team for "primitive floats maintainers", I've added you to the vm-native-maintainers for now, but let's refine this if needed

Indeed; thank you @ejgallego!

And sorry for the annoyance.

No worries! (I think the infrastructure was refactored since my last merge - #14688 (comment))

@erikmd erikmd self-assigned this Nov 3, 2021
@erikmd
Copy link
Member

erikmd commented Nov 3, 2021

@coqbot merge now

@coqbot-app coqbot-app bot merged commit ba27ca7 into coq:master Nov 3, 2021
@proux01 proux01 deleted the deprecate-frexp branch November 4, 2021 09:12
@proux01
Copy link
Contributor Author

proux01 commented Nov 4, 2021

Thanks @erikmd (and thanks @ejgallego )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: cleanup Code removal, deprecation, refactorings, etc. part: primitive types Primitive ints, floats, arrays, etc. part: standard library The standard library stdlib.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants