Skip to content
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

Fix Issue 19312 - Reduce template bloat in std.math by using const arguments #6731

Closed
wants to merge 1 commit into from

Conversation

n8sh
Copy link
Member

@n8sh n8sh commented Oct 18, 2018

Labeling variably-typed arguments of template functions as const causes the same type to be inferred when the argument is const, non-const, and immutable.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @n8sh!

Bugzilla references

Auto-close Bugzilla Severity Description
19312 enhancement Reduce template bloat in std.math by using const arguments

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 + phobos#6731"

@n8sh n8sh force-pushed the issue-19312 branch 5 times, most recently from a0258c8 to cb281d7 Compare October 18, 2018 13:48
@@ -6302,34 +6303,34 @@ version (D_HardFloat) @safe unittest // rounding
* Returns:
* `true` if $(D_PARAM x) is Nan.
*/
bool isNaN(X)(X x) @nogc @trusted pure nothrow
bool isNaN(X)(const X x) @nogc @trusted pure nothrow
Copy link
Member Author

Choose a reason for hiding this comment

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

const here causes an error in CTFE for isNaN(real.nan). Removing the const allows it to work fine. Bizarre.

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that isNaN (along with other functions) is evaluated as a built-in at compile-time and such built-ins are matched by their mangled name in dmd.

Copy link
Member

Choose a reason for hiding this comment

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

Yep. Although some of those builtins are outdated and can (will!) removed at some point, because the real builtins are in core.

@n8sh n8sh closed this Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants