Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

TickDuration UFCS "to" was broken for float #1234

Merged
merged 1 commit into from May 11, 2015

Conversation

burner
Copy link
Member

@burner burner commented Apr 27, 2015

#1190 was not tested throughout, it has a missing "this".

TickDuration t;
auto tf = to!("nsecs",float)(t);

Update:
found this while making the plotting for my benchmark module better

Update2:
best PR number ever :-)

@burner burner force-pushed the TickDuration_to_float_fix branch 9 times, most recently from f04d75d to ba03017 Compare April 28, 2015 22:26
@@ -1703,7 +1702,7 @@ T to(string units, T, D)(D td) @safe pure nothrow @nogc
{
enum unitsPerSec = convert!("seconds", units)(1);

return this.to!("seconds", T)() * unitsPerSec;
return (td.length / cast(T)TickDuration.ticksPerSec) * unitsPerSec;
Copy link
Member

Choose a reason for hiding this comment

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

space after cast(T)

@andralex
Copy link
Member

this needs a fair amount of work

@burner
Copy link
Member Author

burner commented Apr 29, 2015

@jmdavis how can I use monotime with the StopWatch. I found this bug because I was working with the StopWatch.

@andralex indeed this needs works

@jmdavis
Copy link
Member

jmdavis commented Apr 29, 2015

how can I use monotime with the StopWatch. I found this bug because I was working with the StopWatch.

You can't right now, because that would be a breaking change. The benchmarking code in std.datetime would mostly work as-is with MonoTime and Duration instead of TickDuration except for that fact that it's primarily return types that change (so they can't be overloaded). So, those functions are going to need to be replaced with versions using MonoTime and Duration in another module like std.benchmark, at which point, the versions in std.datetime, which use TickDuration can be deprecated along with TickDuration itself, and it should then be easier to deal with the benchmarking functions, since the confusion that is TickDuration won't be needed. Those new functions are on my TODO list, but I haven't managed to find much time for D in the past several months. I think that that will be changing soon, but my primary focus at the moment will be dconf, or I won't have my talk ready in time, so changes like that will almost certainly have to come after. So until I get to it (or someone else does), we unfortunately have to make do with TickDuration when dealing with the benchmarking functions. But we did finally get MonoTime in the last release, so the necessary building blocks are now there.

@burner burner force-pushed the TickDuration_to_float_fix branch 5 times, most recently from 42b225f to 2007a8d Compare April 29, 2015 10:52
@burner
Copy link
Member Author

burner commented Apr 29, 2015

linux_32_64

shared static ~this() groot = (nil)
timed out after 3600 seconds, step failed

not sure what is the cause of this, but the core.time tests pass

dlang#1190 was not tested throughout, it has a missing "this".

TickDuration t;
auto tf = to!("nsecs",float)(t);

unittest fix for win and freebsd based

somehow is(F : float) is true for uint

args

better debugging

another try

another test

another bug dies

size_t

another try

review work

this is becoming sad

...

....

.....

merge
@mrkline
Copy link
Contributor

mrkline commented May 7, 2015

Thanks for really digging into this - my original PR was extremely simple and just hoisted to out of Duration so that it matched the rest of the standard library's semantics. I noticed some of this seemed a bit off, but didn't want to hoist it out and do a rewrite all at once. My apologies for any resulting kerfuffles.

assert(t4f - (cast(F)(t1v - t2v)) < 0.0001);
}
else
foreach (F; _TypeTuple!(int,uint,long,ulong,float,double,real))
{
Copy link
Member

Choose a reason for hiding this comment

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

It's usually better to turn those foreach into a function literal to mitigate the expensive O(N^2) optimizations of the backend.

@MartinNowak
Copy link
Member

Auto-merge toggled on

@burner
Copy link
Member Author

burner commented May 11, 2015

thanks

MartinNowak added a commit that referenced this pull request May 11, 2015
TickDuration UFCS "to" was broken for float
@MartinNowak MartinNowak merged commit 80a2da3 into dlang:master May 11, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants