On Sunday, 26 October 2014 at 22:12:20 UTC, Martin Nowak wrote:
> On 10/25/2014 06:43 PM, Dicebot wrote:
>> Because of that I am going to start voting despite some arguments being
>> still in process. I hope that won't cause any tension.
>
> The dependency on external version identifiers in phobos is still a complete bummer and there are still many implementation issues.
> One reason why this is taking so long is that there were many issues some of which still need to be addressed.

I don't consider it a major issue as I don't think std.logger should be used inside Phobos at all.

On Sunday, 26 October 2014 at 23:45:56 UTC, Martin Nowak wrote:
> On 10/26/2014 11:29 PM, Robert burner Schadek wrote:
>>
>> And I forgot to add, no better solution presented itself in one year.
>
> Well I showed one solution, but reduce it to its essence.
> If you allow to define a Logger with a LogLevel know at compile time and you statically pass the LogLevel of your message to the logging function you can elide that call. For anything else you need a runtime check.
>
> http://dpaste.dzfl.pl/2538c3b5d287

And again I'm saying fixing the LogLevel at CT is not good enough. And the other part of the solution uses class just like std.logger.
And the hierarchy you're building is also at CT, which is just not gone work, if you don't have ultimate control of all sources.

On Sunday, 26 October 2014 at 23:58:14 UTC, Martin Nowak wrote:
> On 10/27/2014 12:45 AM, Martin Nowak wrote:
>> If you allow to define a Logger with a LogLevel know at compile time and
>> you statically pass the LogLevel of your message to the logging function
>> you can elide that call. For anything else you need a runtime check.
>
> You are trying to globally define a LogLevel through the version identifier but that collides with D's separate compilation.
> So you cannot enable logging in a library that was compiled with StdLoggerDisableLogging. And vice versa you cannot statically disable logging in a library compiled without StdLoggerDisableLogging.
>
> Now if you use StdLoggerDisableLogging in your program the effect on the library will depend on whether or not you're calling a templated function or if the compiler inlined certain library function into your program.

It is a good think then that the *DisableLogging versions are only used inside a template that is used inside a templates. Though version statements attached to a phobos compilation should only have impact on the unittest of phobos.

Secondly, why would phobos be shipped with certain LogLevel disabled.

On Monday, 27 October 2014 at 07:03:11 UTC, Dicebot wrote:
>
> I don't consider it a major issue as I don't think std.logger should be used inside Phobos at all.

Yes, using std.logger inside of phobos is a no-no

On Sunday, 26 October 2014 at 22:57:51 UTC, Martin Nowak wrote:
> On 10/26/2014 11:27 PM, Robert burner Schadek wrote:
>> it is not really a dependency as the one template that uses the version
>> identifier uses them optionally.
>
> It simply doesn't work, e.g. you could not statically disable logging in phobos without recompiling phobos.
>
> cat > lib.d << CODE
> version (StdLoggerDisableLogging)
>     enum isLoggingActive = false;
> else
>     enum isLoggingActive = true;
>
> void doSome()
> {
>     import std.stdio;
>     writeln("isLoggingActive: ", isLoggingActive);
> }
> CODE
>
> cat > main.d << CODE
> import lib, std.stdio;
>
> void main()
> {
>     writeln("isLoggingActive: ", isLoggingActive);
>     doSome();
> }
> CODE
>
> dmd -version=StdLoggerDisableLogging -lib lib.d
> dmd main lib.a
> ./main

If it where done this way, yes of course you're right. But it is not, please take a look a the source first.

On Monday, 27 October 2014 at 08:23:48 UTC, Robert burner Schadek wrote:
> On Monday, 27 October 2014 at 07:03:11 UTC, Dicebot wrote:
>>
>> I don't consider it a major issue as I don't think std.logger should be used inside Phobos at all.
>
> Yes, using std.logger inside of phobos is a no-no

Ayway, let's come with an agreement/compromise with Martin and I'll start voting immediately after.

On Monday, 27 October 2014 at 12:03:33 UTC, Dicebot wrote:
>
> Ayway, let's come with an agreement/compromise with Martin and I'll start voting immediately after.

Well, as far as I can see his argument was based on old code that has long been rewritten and he hasn't answered since I pointed that out.

On 10/27/2014 09:08 AM, Robert burner Schadek wrote:
>
> And again I'm saying fixing the LogLevel at CT is not good enough. And
> the other part of the solution uses class just like std.logger.

Right, a CT LogLevel only works for certain use-cases and I never claimed that it works for everything (that's why there is the loggerObject adapter). This not an argument to go solely with classes though. And you loose an important optimization by not making use of CT LogLevels.
Introducing the concepts and providing the loggerObject adapter (structs to interface) is a very flexible solution that proved itself very successful for ranges.

> And the hierarchy you're building is also at CT, which is just not gone
> work, if you don't have ultimate control of all sources.

What's the problem here, a counterexample would be helpful.

Logger logger;
if (config.alsoLogToFile)
    logger = loggerObject(multiLogger(SysLogger(), FileLogger()));
else
    logger = loggerObject(SysLogger());

On 10/27/2014 09:22 AM, Robert burner Schadek wrote:
>
> It is a good think then that the *DisableLogging versions are only used
> inside a template that is used inside a templates. Though version
> statements attached to a phobos compilation should only have impact on
> the unittest of phobos.
>
> Secondly, why would phobos be shipped with certain LogLevel disabled.

Well the question is, how can you avoid paying the overhead for logging when you don't need it.

Say I want to add tracing/logging to [`parseJson`](http://dlang.org/library/std/json/parseJSON.html) or [`findRoot`](http://dlang.org/phobos/std_numeric.html#.findRoot) then I have no way to get rid of the logLevel overhead with the version identifier and abstract class approach.

When the logLevel is know at CT this overhead can be completely eliminated.
auto json = parseJSON(data, stdoutLogger!(LogLevel.trace)());
auto json = parseJSON(data, stdoutLogger!(LogLevel.warning)());
auto json = parseJSON(data, NullLogger());

On Monday, 27 October 2014 at 20:42:10 UTC, Martin Nowak wrote:
> Say I want to add tracing/logging to [`parseJson`](http://dlang.org/library/std/json/parseJSON.html) or [`findRoot`](http://dlang.org/phobos/std_numeric.html#.findRoot)

This is exactly what is wrong :) Using std.logger inside Phobos itself is a big no. Actually even finding yourself in position where you may want to do it indicates some Phobos design issue.