(Trivial) (DragonFly) Some of the core/stdc/math c-library functions had the 'pure' attribute set by dkgroot · Pull Request #2161 · dlang/druntime

dkgroot · 2018-04-11T15:00:00Z

Phobos Compile Error:
std/math.d(4598): Error: pure function std.math.nearbyint cannot call impure function core.stdc.math.nearbyintl

Issue got apparent when dlang/phobos#6409 was merged.
Note: This fix only references DragonFly specific code

dlang-bot · 2018-04-11T15:00:02Z

Thanks for your pull request and interest in making D better, @dkgroot! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + druntime#2161"

dkgroot · 2018-04-11T15:03:03Z

The pure attribute seems to be scattered around a little here.

Why aren't most or even all of these extern c-library math function calls 'pure'? I do not see how any of them could have side-effects. Maybe i am missing something :-)

BTW: I followed the GLIBC version and matched the ones that had to 'pure' attribute (for now).

n8sh · 2018-04-17T19:09:18Z

@dkgroot The source for which functions are annotated pure is @ibuclaw's comment #2045 (comment):

I'll use gcc as a reference, math functions that set errno are marked as MATHFN_ERRNO

n8sh · 2018-04-17T19:16:53Z

src/core/stdc/math.d

@@ -3378,25 +3378,25 @@ else version(DragonFlyBSD)
    double cbrt(double x);


cbrt not pure?

Added pure to the cbrt function... Thanks for checking !

n8sh · 2018-04-17T19:22:35Z

src/core/stdc/math.d

-    real fmaxl(real x, real y);
-    real fminl(real x, real y);
+    pure real fdiml(real x, real y);
+    pure real fmaxl(real x, real y);


Also fmax and fmaxf

n8sh · 2018-04-17T19:25:09Z

src/core/stdc/math.d

-    real fdiml(real x, real y);
-    real fmaxl(real x, real y);
-    real fminl(real x, real y);
+    pure real fdiml(real x, real y);


For non-DragonFlyBSD fdiml isn't marked pure. Same for nexttoward, nextafter.

EDIT: correction

n8sh · 2018-04-17T19:25:42Z

src/core/stdc/math.d

+    pure real erfl(real x);
+    pure real erfcl(real x);
+    pure real lgammal(real x);
+    pure real tgammal(real x);


For non-DragonFlyBSD erf is marked pure but not erfc, lgamma, tgamma.

n8sh · 2018-04-17T19:26:52Z

src/core/stdc/math.d

+    pure real fabsl(real x);
+    pure real hypotl(real x, real y);
+    pure real powl(real x, real y);
+    pure real sqrtl(real x);


For non-DragonFlyBSD cbrt and fabs are marked pure but not hypot, pow, sqrt.

n8sh · 2018-04-17T19:27:51Z

src/core/stdc/math.d

+    pure real logbl(real x);
+    pure real modfl(real x, real *iptr);
+    pure real scalbnl(real x, int y);
+    pure real scalblnl(real x, c_long y);


For non-DragonFlyBSD modf is marked pure but not logb, scalb.

n8sh · 2018-04-17T19:29:08Z

src/core/stdc/math.d

+    pure real frexpl(real x, int *exp);
    int ilogbl(real x);
-    real ldexpl(real x, int exp);
+    pure real ldexpl(real x, int exp);


For non-DragonFlyBSD frexp is marked pure but not exp, exp2, expm1, ldexp.

n8sh · 2018-04-17T19:30:53Z

src/core/stdc/math.d

+    pure real atanhl(real x);
+    pure real coshl(real x);
+    pure real sinhl(real x);
+    pure real tanhl(real x);


For non-DragonFlyBSD tanh and asinh are marked pure but not acosh, atanh, cosh, sinh.

n8sh · 2018-04-17T19:31:20Z

src/core/stdc/math.d

+    pure real acosl(real x);
+    pure real asinl(real x);
+    pure real atanl(real x);
+    pure real atan2l(real x, real y);


For non-DragonFlyBSD atan2 is not marked pure

Thanks for checking all of these. I guess i thought i saw a pattern when making the corrections, which was faulty.

dkgroot · 2018-04-18T13:01:46Z

@n8sh Thanks got checking ! Much appreciated.

ibuclaw · 2018-04-20T21:14:06Z

Check whitespace is failing.

…te needed for DragonFly. Caused trouble compiling phobos since dlang/phobos#6409 Phobos Compile Error: std/math.d(4598): Error: `pure` function `std.math.nearbyint` cannot call impure function `core.stdc.math.nearbyintl`

dkgroot · 2018-04-22T09:18:24Z

Thanks everyone !

dkgroot requested review from ibuclaw and schveiguy as code owners April 11, 2018 15:00

n8sh reviewed Apr 17, 2018

View reviewed changes

dkgroot force-pushed the fix_core_stdc_math branch from 3b28cb8 to 8a70692 Compare April 21, 2018 12:51

ibuclaw approved these changes Apr 21, 2018

View reviewed changes

ibuclaw added the auto-merge label Apr 21, 2018

dlang-bot merged commit b2dc234 into dlang:master Apr 21, 2018

dkgroot added a commit to dkgroot-ldc/druntime that referenced this pull request May 7, 2018

Backport dlang#2161

2d2c365

dkgroot mentioned this pull request May 7, 2018

[Trivial] Backport dlang_druntime PR 2161 ldc-developers/druntime#140

Closed

		@@ -3378,25 +3378,25 @@ else version(DragonFlyBSD)
		double cbrt(double x);

Uh oh!

Comments

Conversation

dkgroot commented Apr 11, 2018

Uh oh!

dlang-bot commented Apr 11, 2018

Bugzilla references

Testing this PR locally

Uh oh!

dkgroot commented Apr 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

n8sh commented Apr 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

n8sh Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkgroot commented Apr 18, 2018

Uh oh!

ibuclaw commented Apr 20, 2018

Uh oh!

dkgroot commented Apr 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dkgroot commented Apr 11, 2018 •

edited

Loading

n8sh Apr 17, 2018 •

edited

Loading