-
Notifications
You must be signed in to change notification settings - Fork 419
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
Rename handful of BigInteger functions and deprecate get_str #22818
Conversation
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
3cf2e61
to
637762d
Compare
Signed-off-by: Jade Abraham <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologize for the nit picking on the documentation! Looks good for the most part.
// private method | ||
@chpldoc.nodoc | ||
proc getStr(base: int = 10): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make this a private proc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That unfortunately requires it no longer being a method
modules/standard/BigInteger.chpl
Outdated
if compiledForSingleLocale() { | ||
var tmpvar = chpl_gmp_mpz_get_str(base_, this.mpz); | ||
|
||
try! { | ||
ret = string.createAdoptingBuffer(tmpvar); | ||
} | ||
|
||
} else if this.localeId == chpl_nodeID { | ||
if compiledForSingleLocale() || this.localeId == chpl_nodeID { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the compiledForSingleLocale()
is a param
, so that is why these were separated out into different branches. It's difficult to say if that is important, but that's why this was here originally. Now, in single locale configurations, we will have the branch and construction of the locale object since we can't fold that out, which can cause some overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did test compiling this in a single locale configuration and it does not generate the extra check (looking at AST for 'codegen' pass). It is short enough code that I can also just put it back
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Removes almost all of the remaining BigInteger deprecations This PR removes the deprecations from the following PRs I authored - #22775 - #22888/#22774 - #22818 - #22794 - #22788 - #22121 I also removed the remaining deprecations done by Yash Raj - #18855 - #18827 I moved one test out of the deprecated/BigInteger directory, `bigint_getlimbs.chpl`, since its not a deprecation test and actually checks functionality. Testing - [x] paratest without comm - [x] paratest with comm - [x] built docs and checked them [Reviewed by @ShreyasKhandekar]
Summary of changes
scan0
/scan1
tofindNext0
/findNext1
and update its documentationpopcount
topopCount
and update its documentationior
toor
and update its argument names and documentationrootrem
/sqrtrem
torootRem
/sqrtRem
and update its argument names and documentationaddmul
/submul
toaddMul
/subMul
and update its argument names and documentationget_str
in favor of casting to astring
or usingwriteThis
scan0
andscan1
New tests
test/deprecated/BigInteger/scan01.chpl
test/deprecated/BigInteger/ior.chpl
test/deprecated/BigInteger/popcount.chpl
test/deprecated/BigInteger/rootSqrtRem.chpl
test/deprecated/BigInteger/addSubMul.chpl
test/deprecated/BigInteger/deprecateGet_str.chpl
test/library/standard/BigInteger/getStr.chpl
test/deprecated/BigInteger/deprecateScanArg.chpl
Testing
[Reviewed by @bmcdonald3]
closes #17737
closes #19389
implements remaining topics from #17724, closes #17724
closes cray/chapel-private#5097