June 15, 2012
This is the std.typecons.Nullable.get() method:

@property ref inout(T) get() inout pure @safe
{
    enforce(!isNull);
    return _value;
}



And this is the std.exception.enforce() and bailOut():

T enforce(T, string file = __FILE__, size_t line = __LINE__)
    (T value, lazy const(char)[] msg = null) @safe pure
{
    if (!value) bailOut(file, line, msg);
    return value;
}

private void bailOut(string file, size_t line, in char[] msg) @safe pure
{
    throw new Exception(msg ? msg.idup : "Enforcement failed", file, line);
}



What do you think about replacing the bailOut() with:


version (halting_enforce)
{
    private void bailOut(string file, size_t line, in char[] msg) @safe pure nothrow
    {
        assert(0, msg ? msg : "Enforcement failed");
    }
}
else
{
    private void bailOut(string file, size_t line, in char[] msg) @safe pure
    {
        throw new Exception(msg ? msg.idup : "Enforcement failed", file, line);
    }
}


Even if this version is not documented in Phobos docs (because the inliner someday will be better), today it allows methods that must be fast like Nullable.get() to be inlined (but I have not tested this), regaining the lost performance.

Bye,
bearophile
June 15, 2012
On 15.06.2012 16:17, bearophile wrote:
>
> What do you think about replacing the bailOut() with:
>
>
> version (halting_enforce)
> {
> private void bailOut(string file, size_t line, in char[] msg) @safe pure
> nothrow
> {
> assert(0, msg ? msg : "Enforcement failed");
> }
> }
> else
> {
> private void bailOut(string file, size_t line, in char[] msg) @safe pure
> {
> throw new Exception(msg ? msg.idup : "Enforcement failed", file, line);
> }
> }

Stupidity. An exemplar one.
Anyone caught trying to hijacking core exception handling primive (that by the way check things like file I/O) in std library should be shot on sight.


-- 
Dmitry Olshansky
June 15, 2012
On 15.06.2012 16:17, bearophile wrote:
>
> Even if this version is not documented in Phobos docs (because the
> inliner someday will be better), today it allows methods that must be
> fast like Nullable.get() to be inlined (but I have not tested this),
> regaining the lost performance.
>

If that is sole concern, I suggest do a pull that makes enforce vs assert decision accessible from the outside:

NotNull!(T, NotNull.Strict) //asserts
NotNull!T //throws


-- 
Dmitry Olshansky
June 15, 2012
On Friday, 15 June 2012 at 12:17:32 UTC, bearophile wrote:
> Even if this version is not documented in Phobos docs (because the inliner someday will be better), today it allows methods that must be fast like Nullable.get() to be inlined (but I have not tested this), regaining the lost performance.

And how the assert helps with inlining?
June 15, 2012
On 2012-06-15 14:55, Kagamin wrote:
> On Friday, 15 June 2012 at 12:17:32 UTC, bearophile wrote:
>> Even if this version is not documented in Phobos docs (because the
>> inliner someday will be better), today it allows methods that must be
>> fast like Nullable.get() to be inlined (but I have not tested this),
>> regaining the lost performance.
>
> And how the assert helps with inlining?

Perhaps throwing an exception prevents inlining ?

-- 
/Jacob Carlborg
June 15, 2012
Dmitry Olshansky:

> If that is sole concern, I suggest do a pull that makes enforce vs assert decision accessible from the outside:
>
> NotNull!(T, NotNull.Strict) //asserts
> NotNull!T //throws

Nullable isn't the only problem, there are similar problems with
std.random.uniform. How do you tell uniform() what kind of
Nullable to use?

---------------------

> Perhaps throwing an exception prevents inlining ?

With DMD.

Bye,
bearophile
June 15, 2012
On 15.06.2012 18:27, bearophile wrote:
> Dmitry Olshansky:
>
>> If that is sole concern, I suggest do a pull that makes enforce vs
>> assert decision accessible from the outside:
>>
>> NotNull!(T, NotNull.Strict) //asserts
>> NotNull!T //throws
>
> Nullable isn't the only problem, there are similar problems with
> std.random.uniform. How do you tell uniform() what kind of
> Nullable to use?

why would uniform have to do anything with nullable?
If anything it's handled like always via tempaltes.


-- 
Dmitry Olshansky
June 15, 2012
On Friday, June 15, 2012 15:50:17 Jacob Carlborg wrote:
> On 2012-06-15 14:55, Kagamin wrote:
> > On Friday, 15 June 2012 at 12:17:32 UTC, bearophile wrote:
> >> Even if this version is not documented in Phobos docs (because the
> >> inliner someday will be better), today it allows methods that must be
> >> fast like Nullable.get() to be inlined (but I have not tested this),
> >> regaining the lost performance.
> > 
> > And how the assert helps with inlining?
> 
> Perhaps throwing an exception prevents inlining ?

It's the fact that enforce is lazy which prevents inlining (which really does need to be fixed or enforce is going to continue to be a performance problem).

However, this suggestion is clearly bad, because it's suggesting turning an exception into an assertion, which is _very_ broken thing to do. Assertions and exceptions are two _very_ different things and should be treated as such.

- Jonathan M Davis
June 15, 2012
Jonathan M Davis:

> It's the fact that enforce is lazy which prevents inlining (which really does
> need to be fixed or enforce is going to continue to be a performance problem).

Then my "solution" solves nothing. Thank you Jonathan.


> However, this suggestion is clearly bad, because it's suggesting turning an
> exception into an assertion, which is _very_ broken thing to do. Assertions
> and exceptions are two _very_ different things and should be treated as such.

enforce() sometimes is used as an assert you can't disable (as in Nullable.get, I think). So maybe such cases are better handled introducing two levels of asserts, "light asserts" and "strong asserts".

Bye,
bearophile
June 15, 2012
On 15-06-2012 19:31, bearophile wrote:
> Jonathan M Davis:
>
>> It's the fact that enforce is lazy which prevents inlining (which
>> really does
>> need to be fixed or enforce is going to continue to be a performance
>> problem).
>
> Then my "solution" solves nothing. Thank you Jonathan.
>
>
>> However, this suggestion is clearly bad, because it's suggesting
>> turning an
>> exception into an assertion, which is _very_ broken thing to do.
>> Assertions
>> and exceptions are two _very_ different things and should be treated
>> as such.
>
> enforce() sometimes is used as an assert you can't disable (as in
> Nullable.get, I think). So maybe such cases are better handled
> introducing two levels of asserts, "light asserts" and "strong asserts".
>
> Bye,
> bearophile

No, Jonathan's point is that enforce() can be used with both Errors and Exceptions. When used with Errors, we're talking about logic errors (effectively the same as an assert, with the exception that it isn't compiled out in release builds), while when we're talking about Exceptions, the error is recoverable. See for example some uses of enforce() in std.stdio.File.

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
« First   ‹ Prev
1 2
Top | Discussion index | About this forum | D home