Skip to content

std.math: №2, fix abs, fabs#3012

Merged
burner merged 2 commits intodlang:masterfrom
9il:abs
Mar 5, 2015
Merged

std.math: №2, fix abs, fabs#3012
burner merged 2 commits intodlang:masterfrom
9il:abs

Conversation

@9il
Copy link
Member

@9il 9il commented Feb 20, 2015

intrinsics for fabs should be added to DMD.

intrinsic for `fabs` should be added to DMD.

fix unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to be picky, but afaik there should be no FIXME in a pull request.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I have already pushed scores of FIXMEs into phobos. This is normal practise if DMD should be changed first.

Copy link
Member

Choose a reason for hiding this comment

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

Fix what?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fix code like

double fabs(double x) @safe pure nothrow @nogc { return fabs(cast(real)x); }

to declaration

double fabs(double x) @safe pure nothrow @nogc;

when DMD implement this intrinsic.

@kuettler
Copy link
Contributor

There is fabsl, fabs and fabsf in core.stdc.math whenever available. Maybe I just do not understand what you are trying to do.

@9il
Copy link
Member Author

9il commented Feb 20, 2015

This PRs have few goals:

  1. fix dmd tests where code like fabs(-2) is used
  2. fix unittest in phobos (same code style)
  3. allow fabs and other intrinsics to be replaced in other compilers (like LDC)
  4. allow to write other functions based on correct API: float log(float), double exp(double), double gamma(double).

This isn't change for fabs itself, but for other functions and compilers.

@9il
Copy link
Member Author

9il commented Feb 20, 2015

PS: core.stdc.math is not pure.

@burner
Copy link
Member

burner commented Mar 4, 2015

Could you add a unittest that calls all three version, simular to what is done in std.string

foreach (S; TypeTuple!(float, double, real))

@9il
Copy link
Member Author

9il commented Mar 5, 2015

Unittest was added.

@burner
Copy link
Member

burner commented Mar 5, 2015

Thank, LGTM

@burner
Copy link
Member

burner commented Mar 5, 2015

Auto-merge toggled on

burner added a commit that referenced this pull request Mar 5, 2015
std.math: №2, fix abs, fabs
@burner burner merged commit 0af21d7 into dlang:master Mar 5, 2015
@9il 9il deleted the abs branch March 5, 2015 12:38
@9rnsr
Copy link
Contributor

9rnsr commented Jul 29, 2015

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=14842

@9il
Copy link
Member Author

9il commented Jul 29, 2015

... and this is good because implementation bugs like in approxEqual can be fixed faster both in Phobos and user code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants