Skip to content

Comments

std.math: №1, change protection#3011

Merged
DmitryOlshansky merged 1 commit intodlang:masterfrom
9il:float-traits
Jul 15, 2015
Merged

std.math: №1, change protection#3011
DmitryOlshansky merged 1 commit intodlang:masterfrom
9il:float-traits

Conversation

@9il
Copy link
Member

@9il 9il commented Feb 20, 2015

Attribute was changed for RealFormat, floatTraits, floorImpl. This
suff is usefull (#3045) in other modules like std.numeric and
std.mathspecial.

Attribute changed for `RealFormat`, `floatTraits`, `floorImpl`. This
suff is usefull in other modules like `std.numeric` and
`std.mathspecial`.
@kuettler
Copy link
Contributor

Hmm. This is exposing implementation details, aka enlarging the module's public interface for increased coupling. I guess the newly exposed code needs to be reviewed here. This is not a one line change. ;)

@9il
Copy link
Member Author

9il commented Feb 20, 2015

You are welcome;) If you like numeric algorithms see also #2991. And if you have Windows please help with #2548

@JohanEngelen
Copy link
Contributor

@9il Perhaps you can limit the changes to only add RealFormat and floatTraits to the public interface? (i.e. keep floorImpl and MANTISSA_LSB private)
Hope that expedites the merge.

@9il
Copy link
Member Author

9il commented May 1, 2015

@JohanEngelen floorImpl is useful for optimization.
Programmer can use it when he handle special cases directly.

float floor(float x) @trusted pure nothrow @nogc
{
    // Special cases.
    if (isNaN(x) || isInfinity(x) || x == 0.0)
        return x;

    return floorImpl(x);
}

@quickfur
Copy link
Member

ping

Is there anything else to do before merging this?

@9il
Copy link
Member Author

9il commented Jun 30, 2015

looks like nothing

@ibuclaw
Copy link
Member

ibuclaw commented Jun 30, 2015

Can we only expose RealFormat and FloatTraits for now, leaving the MANTISSA_LSB/MSB and floorImpl private unless we really need them later down the line.

@9il
Copy link
Member Author

9il commented Jun 30, 2015

I think conceptually all this stuff may be package because we have mathspecial.

@9il
Copy link
Member Author

9il commented Jul 2, 2015

For example we need floorImpl for future Bessel functions at std.mathspecial.

@DmitryOlshansky
Copy link
Member

package is not public so I don't see any problem with this pull.

@DmitryOlshansky
Copy link
Member

Auto-merge toggled on

@9il
Copy link
Member Author

9il commented Jul 15, 2015

Thanks!

DmitryOlshansky added a commit that referenced this pull request Jul 15, 2015
std.math: №1, change protection
@DmitryOlshansky DmitryOlshansky merged commit db4d872 into dlang:master Jul 15, 2015
@9il 9il deleted the float-traits branch July 20, 2015 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants