D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6586 - feqrel for const values too
Summary: feqrel for const values too
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P2 enhancement
Assignee: No Owner
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2011-08-31 22:09 UTC by bearophile_hugs
Modified: 2017-07-19 17:42 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description bearophile_hugs 2011-08-31 22:09:28 UTC
import std.math: feqrel;
void main() {
    double x = 1.5;
    const double y = 1.50000001;
    auto d = feqrel(x, y);
}


DMD 2.055head gives me:

test.d(5): Error: template std.math.feqrel(X) if (isFloatingPoint!(X)) does not match any function template declaration
test.d(5): Error: template std.math.feqrel(X) if (isFloatingPoint!(X)) cannot deduce template function from argument types !()(double,const(double))


I'd like feqrel to work with mixed const arguments too. Const doubles come from "in" function arguments, and they pop out in other situations too, like:


import std.math: feqrel;
double foo(double x)
out(r) {
    assert(feqrel(r, x) > 10);
}
body {
    return x + 0.0000001;
}
void main() {
    foo(1.5);
}


Workaround:


import std.math: feqrel;
double foo(double x)
out(r) {
    assert(feqrel(cast()r, x) > 10);
}
body {
    return x + 0.0000001;
}
void main() {
    foo(1.5);
}




To solve this problem I think that it's enough to replace this:


int feqrel(X)(X x, X y) @trusted pure nothrow
    if (isFloatingPoint!(X))
{
    /* Public Domain. Author: Don Clugston, 18 Aug 2005.
     */


With something like:

int feqrel(X1, X2)(X1 x, X2 y) @trusted pure nothrow
    if (isFloatingPoint!(Unqual!X1) && is(Unqual!X1 == Unqual!X2))
{
    /* Public Domain. Author: Don Clugston, 18 Aug 2005.
     */
    alias Unqual!X1 X;
Comment 1 Don 2011-09-01 12:06:31 UTC
I hit this myself a couple of days ago.
I prefer:

int feqrel(X)(const(X) x, const(X) y) @trusted pure nothrow
    if (isFloatingPoint!(X))

which causes less template bloat, and also avoids some unpleasant issues where x and y are different sizes.

I'm not really sure why double and const(double) need to be different types. It causes a huge amount of template bloat, and I don't think we're getting much (if anything) in return.
Comment 2 bearophile_hugs 2011-09-02 03:18:39 UTC
(In reply to comment #1)

> I'm not really sure why double and const(double) need to be different types. It
> causes a huge amount of template bloat, and I don't think we're getting much
> (if anything) in return.

Tell this again in the main D newsgroup :-)
Comment 3 hsteoh 2014-04-08 13:23:49 UTC
Tested on git HEAD: the original code now compiles, although I did find a different related failing case:

======Code:======
import std.math, std.stdio;
void main() {
        const(double) a=1.0, b=2.0;
        writeln(feqrel(a, b));
}
======Compile:======
/usr/src/d/phobos/std/math.d(5394): Error: cannot modify const expression diff
test.d(4): Error: template instance std.math.feqrel!(const(double)) error instantiating
Comment 4 github-bugzilla 2015-02-27 16:30:46 UTC
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/09ebc581ec022d0e1091d47887aab26366df79e0
rework and try to fix Issue 6586

https://github.com/D-Programming-Language/phobos/commit/62acd72cec052169e7eb56d1c63c860ae92f9ef9
Merge pull request #3017 from 9il/frexp

[2.067.0-b2][regression] fix Issues 14212 and 6586
Comment 5 github-bugzilla 2015-02-27 20:55:47 UTC
Commit pushed to 2.067 at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/cf57c7529a63ac970167c65cb659d5249a3a1ab9
Merge pull request #3017 from 9il/frexp

[2.067.0-b2][regression] fix Issues 14212 and 6586
Comment 6 github-bugzilla 2015-04-11 12:24:50 UTC
Commit pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/cf57c7529a63ac970167c65cb659d5249a3a1ab9
Merge pull request #3017 from 9il/frexp