Skip to content

fix Issue 13662: @safe pure nothrow @nogc findRoot#2639

Merged
MartinNowak merged 1 commit intodlang:masterfrom
9il:findroot
Nov 7, 2014
Merged

fix Issue 13662: @safe pure nothrow @nogc findRoot#2639
MartinNowak merged 1 commit intodlang:masterfrom
9il:findroot

Conversation

@9il
Copy link
Member

@9il 9il commented Oct 24, 2014

Reason: allows findRoot be used in @safe @nogc nothrow code.

@9il 9il changed the title more generic findRoot generic findRoot Oct 24, 2014
@MartinNowak
Copy link
Member

Can you please file a bugzilla issue for the @safe @nogc nothrow thing.
Among other things this allows us to automatically mention this in the changelog.

std/numeric.d Outdated
Copy link
Member

Choose a reason for hiding this comment

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

You should add an overload to make that work again or this change will break code.
Maybe it's possible to deprecate the overload at some point.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no Idea how to make overload =(

This unittest was written by me for regression control (allow different R and T types in the same time). I think nobody use this style.

Copy link
Member

Choose a reason for hiding this comment

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

Well just add a function with the old signature and forward it's implementation to the new find root.

@9il 9il changed the title generic findRoot fix Issue 13662: @safe pure nothrow @nogc findRoot Oct 27, 2014
std/numeric.d Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

copy-past:

I have no Idea how to make overload =(

This unittest was written by me for regression control (allow different R and T types in the same time). I think nobody use this style.

Choose a reason for hiding this comment

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

What is the problem with this one? Why it doesn't work anymore?

Choose a reason for hiding this comment

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

I see, link to original question : #2639 (comment)

Choose a reason for hiding this comment

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

I am actually not sure why compiler can't find matching overload here, looks unambiguous.

Copy link
Member

Choose a reason for hiding this comment

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

The template signature changed, so we need to add an overload with the old signature.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I add an overload for old signature than findRoot won't be compiled in most cases because there will be 2 suitable templates .

@quickfur
Copy link
Member

quickfur commented Nov 6, 2014

ping
Any updates based on comments?

@9il
Copy link
Member Author

9il commented Nov 6, 2014

no idea =(

@9il
Copy link
Member Author

9il commented Nov 6, 2014

I think we can push this commit as is or create something like findRootOld.

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the default argument work here?
Maybe in combination with a default type for DT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will try.

Copy link
Member Author

Choose a reason for hiding this comment

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

Compatible for delegates only, but not for other callable entities (functions, structs, objects).

@MartinNowak
Copy link
Member

So please add back the old signature as overload.

T findRoot(T, R)(scope R delegate(T) f, in T a, in T b,
    scope bool delegate(T lo, T hi) tolerance = (T a, T b) => false)
{
    return find root!(T, typeof(f), typeof(tolerance))(f, a, b, tolerance);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

moved

@9il
Copy link
Member Author

9il commented Nov 6, 2014

rebased

@9il
Copy link
Member Author

9il commented Nov 6, 2014

Thanks @MartinNowak. This overload works (I thought it wouldn't work.)!

MartinNowak added a commit that referenced this pull request Nov 7, 2014
fix Issue 13662: @safe pure nothrow @nogc findRoot
@MartinNowak MartinNowak merged commit 26b28ec into dlang:master Nov 7, 2014
@9il 9il deleted the findroot branch November 7, 2014 06:36
@9il 9il mentioned this pull request Apr 28, 2016
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.

4 participants