(2010/08/21 5:04), Andrei Alexandrescu wrote:
> I think the code is ready for prime time, modulo the issues below. What do you all think?
>
> Overall this is a good example of modern, idiomatic D code. Everything is clear, simple, and in the right place. Congratulations, Shoo!
>
> *******
>

> Line 80: You could an assert or even an enforce here for TICKSPERSPEC.
>

A program does not work at all when a function failed in a module constructor. I'll set 0 for TICKSPERSEC when a function failed. And, had better I make TICKSPERSEC -> TicksPerSec?


 > Line 737: I'm afraid you can't put @trusted here because you don't know
 > the safety level of BaseFunc and TargetFunc. You'll need to use @system.
 >

This is a source of worry very much.
I hit on the idea that manage to do:

private @safe void dummySafeFunc(alias FN)()
{
	FN();
}

template isSafe(alias FN)
{
	enum isSafe = is(typeof({dummySafeFunc!(FN)();}()));
}
@safe
ComparingBenchmarkReturnValue comparingBenchmark(
     alias baseFunc, alias targetFunc, int CNT = 0xfff)()
     if (isSafe!baseFunc && isSafe!targetFunc)
{
....
}

@system
ComparingBenchmarkReturnValue comparingBenchmark(
     alias baseFunc, alias targetFunc, int CNT = 0xfff)()
     if (!(isSafe!baseFunc && isSafe!targetFunc))
{
....
}

It is dirty slightly...

May I put isSafe in std.traits?


> Line 21: please add a comment that you're adding this "import inside struct" curiosity with an experimental purpose only.
>
> Line 99: I'm a bit worried that we allow toSeconds for all integer widths. Probably if (isIntegral!T && T.sizeof >= 4) would be better.
>
> Line 170: same concern about toMilliseconds - even bigger because there are lots more milliseconds out there :o).
>
> Line 197: the parameter name should be msec
>
> Line 218: same discussion about the allowed integral types
>
> Line 245: parameter name
>
> Line 431: "Unused"
>
> Line 469: typedef is deprecated (sorry). That's partly why I'm
> suggesting to go with the enum.
>
> Line 739: Since the two aliases are actually functions, you may want to start their names with lowercase.
>

I see.

> Line 762: Beautiful idiom!
>

Thanks! But, I want to write like this:
with (measureTime!((a){assert(a.seconds);}))
{
     doSomething();
}

I don't want to name a nonuse variable.
In this usage, I think that With is proper.
But, this doesn't call destructor. (for bug 3516?
Though I think that how to use is free.

On 08/20/2010 05:47 PM, SHOO wrote:
> (2010/08/21 5:04), Andrei Alexandrescu wrote:
>> I think the code is ready for prime time, modulo the issues below. What do you all think?
>>
>> Overall this is a good example of modern, idiomatic D code. Everything is clear, simple, and in the right place. Congratulations, Shoo!
>>
>> *******
>>
>
>> Line 80: You could an assert or even an enforce here for TICKSPERSPEC.
>>
>
> A program does not work at all when a function failed in a module constructor. I'll set 0 for TICKSPERSEC when a function failed. And, had better I make TICKSPERSEC -> TicksPerSec?

ticksPerSec :o)

>  > Line 737: I'm afraid you can't put @trusted here because you don't know
>  > the safety level of BaseFunc and TargetFunc. You'll need to use @system.
>  >
>
> This is a source of worry very much.
> I hit on the idea that manage to do:
>
> private @safe void dummySafeFunc(alias FN)()
> {
> FN();
> }
>
> template isSafe(alias FN)
> {
> enum isSafe = is(typeof({dummySafeFunc!(FN)();}()));
> }
> @safe
> ComparingBenchmarkReturnValue comparingBenchmark(
> alias baseFunc, alias targetFunc, int CNT = 0xfff)()
> if (isSafe!baseFunc && isSafe!targetFunc)
> {
> ....
> }

I think that's a very good idea. In the long term we'll need to have some traits (either compiler-provided or deduced as above) and put them in std.traits.

> @system
> ComparingBenchmarkReturnValue comparingBenchmark(
> alias baseFunc, alias targetFunc, int CNT = 0xfff)()
> if (!(isSafe!baseFunc && isSafe!targetFunc))
> {
> ....
> }
>
> It is dirty slightly...
>
> May I put isSafe in std.traits?

Yes please.

>> Line 762: Beautiful idiom!
>>
>
> Thanks! But, I want to write like this:
> with (measureTime!((a){assert(a.seconds);}))
> {
> doSomething();
> }
>
> I don't want to name a nonuse variable.
> In this usage, I think that With is proper.
> But, this doesn't call destructor. (for bug 3516?
> Though I think that how to use is free.

It's definitely a bug. But note that in C++ an unused value will be destroyed at the end of expression, which makes sense. That means your idiom would need a named variable.


Andrei

On Friday, August 20, 2010 18:11:53 Andrei Alexandrescu wrote:
> >> Line 762: Beautiful idiom!
> > 
> > Thanks! But, I want to write like this:
> > with (measureTime!((a){assert(a.seconds);}))
> > {
> > doSomething();
> > }
> > 
> > I don't want to name a nonuse variable.
> > In this usage, I think that With is proper.
> > But, this doesn't call destructor. (for bug 3516?
> > Though I think that how to use is free.
> 
> It's definitely a bug. But note that in C++ an unused value will be destroyed at the end of expression, which makes sense. That means your idiom would need a named variable.

It is a very good idiom though.

- Jonathan M Davis