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

precompile: support @big_str parsed integers #27144

Closed
wants to merge 1 commit into from

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented May 18, 2018

In support of #26991 (enable __precompile__ flag by default), this ensures that parser-derived "big" numbers are restored completely at runtime, to remove that odd "gotcha".

@JeffBezanson
Copy link
Sponsor Member

This is clever, but I don't feel it is sufficient. We should be able to handle non-literal bigints. Otherwise there are still surprises like big"1" working but big(1) not, as well as not supporting things like generating constant tables.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented May 18, 2018

In my experience, the issues in question have always been that literals are broken under pre-compilation. That pre-allocating constant objects doesn't necessarily quite work the same is well known and doesn't seem to come up often.

@kshyatt kshyatt added the compiler:precompilation Precompilation of modules label May 28, 2018
@vtjnash vtjnash changed the title precompile: support @big_str parsed numbers precompile: support @big_str parsed integers Jul 13, 2018
And switch `big` macro to throwing errors immediately
(when we can give perfect line info),
rather than deferring until until maybe at runtime
@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jul 16, 2018

OK to merge? I'm hoping / expecting we'll modify GMP to solve this completely in the future (like I did recently for MPFR), but this'll at least handle a few cases for now

@StefanKarpinski
Copy link
Sponsor Member

@JeffBezanson? This seems better than nothing and it already works.

@JeffBezanson
Copy link
Sponsor Member

I don't really think it's worth it --- it doesn't fix cases like #15722 or the use in stdlib/Random.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jul 20, 2018

Alright, lacking any helpful counter-proposal, I'll kill this for now

@vtjnash vtjnash closed this Jul 20, 2018
@vtjnash vtjnash deleted the jn/precompile-bigint branch July 20, 2018 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:precompilation Precompilation of modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants