November 01, 2014
There is still a critical issue with std.experimental.logger that would prevent it from being merged right now: The abuse of @trusted all over the code.

For example, the following piece of code compiles with burner/logger@c87e1032:
---
import std.experimental.logger.core;

struct Dangerous {
    string toString() {
        *(cast(int*)0xdeadbeef) = 0xcafebabe;
        return null;
    }
}

void main() @safe {
    logf("%s", Dangerous());
}
---

To be quite honest, I'm rather disappointed that I was able to break @safe-ty
with the first, most straightforward piece of code I tried. Sure, many people don't understand how dangerous @trusted applied to templates really is. But this issue has already been pointed out multiple times in Robert's code, both in this pull request and other unrelated ones.

David
November 01, 2014
On Saturday, 1 November 2014 at 17:35:37 UTC, David Nadlinger wrote:
> There is still a critical issue with std.experimental.logger that would prevent it from being merged right now: The abuse of @trusted all over the code.
>
> For example, the following piece of code compiles with burner/logger@c87e1032:
> ---
> import std.experimental.logger.core;
>
> struct Dangerous {
>     string toString() {
>         *(cast(int*)0xdeadbeef) = 0xcafebabe;
>         return null;
>     }
> }
>
> void main() @safe {
>     logf("%s", Dangerous());
> }
> ---
>
> To be quite honest, I'm rather disappointed that I was able to break @safe-ty
> with the first, most straightforward piece of code I tried. Sure, many people don't understand how dangerous @trusted applied to templates really is. But this issue has already been pointed out multiple times in Robert's code, both in this pull request and other unrelated ones.
>
> David

Oh, I have actually completely missed that. It is critical indeed. Will try having a look myself and possibly providing patches.
November 02, 2014
On Saturday, 1 November 2014 at 17:35:37 UTC, David Nadlinger wrote:
> There is still a critical issue with std.experimental.logger that would prevent it from being merged right now: The abuse of @trusted all over the code.

Thank you. I was afraid I'd have to harp on it for the fourth time...

November 02, 2014
On Sunday, 2 November 2014 at 00:29:10 UTC, Jakob Ovrum wrote:
> On Saturday, 1 November 2014 at 17:35:37 UTC, David Nadlinger wrote:
>> There is still a critical issue with std.experimental.logger that would prevent it from being merged right now: The abuse of @trusted all over the code.
>
> Thank you. I was afraid I'd have to harp on it for the fourth time...

Sorry, it is really hard to follow important bits in many discussions spawned :( I'll take a go at @safe-ty issue personally this time.
November 02, 2014
I had initial go at cleaning safety attributes : https://github.com/Dicebot/phobos/tree/logger-fix-trust

It still needs one druntime PR merged (https://github.com/D-Programming-Language/druntime/pull/1009) to be done properly but should be ready for destruction.

David snipet results in this after all changes:
std/experimental/logger/core.d(1675): Error: safe function 'std.experimental.logger.core.Logger.logf!(11, "aaa.d", "aaa.main", "void aaa.main() @safe", "aaa", Dangerous).logf' cannot call system function 'std.format.formattedWrite!(MsgRange, char, Dangerous).formattedWrite'
November 02, 2014
On Sunday, 2 November 2014 at 13:06:24 UTC, Dicebot wrote:
> I had initial go at cleaning safety attributes : https://github.com/Dicebot/phobos/tree/logger-fix-trust
>
> It still needs one druntime PR merged (https://github.com/D-Programming-Language/druntime/pull/1009) to be done properly but should be ready for destruction.
>
> David snipet results in this after all changes:
> std/experimental/logger/core.d(1675): Error: safe function 'std.experimental.logger.core.Logger.logf!(11, "aaa.d", "aaa.main", "void aaa.main() @safe", "aaa", Dangerous).logf' cannot call system function 'std.format.formattedWrite!(MsgRange, char, Dangerous).formattedWrite'

I don't recall off the top of my head some non-template innards actually might require @safe, but apart from that, why not just leave the job to template attribute inference entirely? If somebody wants to log a type with a @system toString in non-@safe code, why not just let them?

David
November 02, 2014
> I don't recall off the top of my head some non-template innards actually might require @safe, but apart from that, why not just leave the job to template attribute inference entirely? If somebody wants to log a type with a @system toString in non-@safe code, why not just let them?
>
> David

In my opinion inference is better choice for small building blocks (like algorithms). For complete system like logging API forcing @safe makes more sense as whatever its internals are, exposed API should never be @system
November 02, 2014
On Sunday, 2 November 2014 at 18:29:09 UTC, Dicebot wrote:
> In my opinion inference is better choice for small building blocks (like algorithms). For complete system like logging API forcing @safe makes more sense as whatever its internals are, exposed API should never be @system

This isn't about the library internals. These should be presented in a safe way, of course. The point here is that you restrict what your users can do by forcing templates to be @safe.

Imagine somebody has a type that cannot be @trusted because of whatever reason. Maybe because it's legacy code, maybe it uses resources it does not manage, … If you forcibly make logf @safe, then this type cannot be used with logf without some crazy workaround (simply using to!string might produce an unneeded allocation if the type uses the sink-delegate signature for toString).

Why not leave this up to the compiler and support more use cases without degrading the experience for @safe clients?

David
November 02, 2014
On Sunday, 2 November 2014 at 18:42:20 UTC, David Nadlinger wrote:
> Imagine somebody has a type that cannot be @trusted because of whatever reason. Maybe because it's legacy code, maybe it uses resources it does not manage, … If you forcibly make logf @safe, then this type cannot be used with logf without some crazy workaround (simply using to!string might produce an unneeded allocation if the type uses the sink-delegate signature for toString).
>
> Why not leave this up to the compiler and support more use cases without degrading the experience for @safe clients?
>
> David

You mean something like user type toString() which is legitimately @system and can't be made @trusted? Yes, this makes sense. Will need to also add tests for that.

Consider me convinced :)
November 03, 2014
On 11/2/14 10:07 PM, Dicebot wrote:
> On Sunday, 2 November 2014 at 18:42:20 UTC, David Nadlinger wrote:
>> Imagine somebody has a type that cannot be @trusted because of
>> whatever reason. Maybe because it's legacy code, maybe it uses
>> resources it does not manage, … If you forcibly make logf @safe, then
>> this type cannot be used with logf without some crazy workaround
>> (simply using to!string might produce an unneeded allocation if the
>> type uses the sink-delegate signature for toString).
>>
>> Why not leave this up to the compiler and support more use cases
>> without degrading the experience for @safe clients?
>>
>> David
>
> You mean something like user type toString() which is legitimately
> @system and can't be made @trusted? Yes, this makes sense. Will need to
> also add tests for that.
>
> Consider me convinced :)

Fantastic, thanks! -- Andrei