Am Mon, 08 Sep 2014 11:17:48 +0000
schrieb "Robert burner Schadek" <rburners@gmail.com>:

> On Saturday, 30 August 2014 at 02:16:55 UTC, Dicebot wrote:
> >
> > ==============================
> > Martin Nowak
> > ==============================
> >
> > "Support duck-typing for the log functions.
> > Logger should be a concept and log functions should be
> > free-standing
> > UFCS functions that take any `isLogger!T`.
> > To support a global `defaultLog` variable, you could add a
> > Logger
> > interface and a loggerObject shim. See
> > http://dlang.org/phobos/std_range.html#inputRangeObject for
> > this a pattern."
> >
> > Neither seem to be addressed nor countered.
> 
> Overly complicated IMO

This may sound surprising, but I believe if we want to make
Phobos consistent and give no incentive to roll your own
stuff, we should do this for a lot of APIs. Without going into
depth (but we could) there are good reasons to use classes and
there are good reasons to use duck typing structs.
Another API where this mixed scheme would apply are streams.
By using function templates with `if (isLogger!T)` and an
abstract class Logger, it will only get instantiated once for
all derived classes reducing template bloat, while allowing
custom instantiations for logger structs to avoid virtual
function calls or GC issues. So I agree with Martin.
It is a great way to bring the two camps together without
major casualties.

-- 
Marco

Am Mon, 08 Sep 2014 11:06:42 +0000
schrieb "Robert burner Schadek" <rburners@gmail.com>:

> > ====================================
> > Francesco Cattoglio
> > ====================================
> >
> > "As far as I undestood, there's no way right now to do logging without using the GC. And that means it is currently impossible to log inside destructor calls. That is a blocker in my book."
> >
> > First part partially addressed - missing @nogc @nothrow logger implementation out of the box. […]
> 
> at least for logf nothrow will not work because of a wrong formatting string or args. log can not be nothrow because custom toString for structs and class are allowed.
> 
> nogc is not possible because of custom toString
> 
> that won't fix, but for default types it is nogc.

It is fairly obvious that the next GC implementation needs to allow allocations during a sweep. Maybe we should just assume that it already works ?

-- 
Marco

On Monday, 8 September 2014 at 12:36:29 UTC, Marco Leise wrote:

>
> This may sound surprising, but I believe if we want to make
> Phobos consistent and give no incentive to roll your own
> stuff, we should do this for a lot of APIs. Without going into
> depth (but we could) there are good reasons to use classes and
> there are good reasons to use duck typing structs.
> Another API where this mixed scheme would apply are streams.
> By using function templates with `if (isLogger!T)` and an
> abstract class Logger, it will only get instantiated once for
> all derived classes reducing template bloat, while allowing
> custom instantiations for logger structs to avoid virtual
> function calls or GC issues. So I agree with Martin.
> It is a great way to bring the two camps together without
> major casualties.

I think the template bloat argument is invalid as __LINE__ and friends are passed as template arguments to allow write and writef type logging.

Anyway I will try to make them free standing

On Monday, 8 September 2014 at 13:20:27 UTC, Robert burner Schadek wrote:
> On Monday, 8 September 2014 at 12:36:29 UTC, Marco Leise wrote:

> I think the template bloat argument is invalid as __LINE__ and friends are passed as template arguments to allow write and writef type logging.
>
> Anyway I will try to make them free standing

The biggest problem I have currently with this that you, or at least I, can not override the free standing function.

void log(L)(ref L logger) if(isLogger!L) { ... } will match always
and if I create void log(L)(ref L logger) if(isMySpecialLogger!L) { ... }
both match and thats a nogo

Am Mon, 08 Sep 2014 13:37:02 +0000
schrieb "Robert burner Schadek" <rburners@gmail.com>:

> On Monday, 8 September 2014 at 13:20:27 UTC, Robert burner Schadek wrote:
> > On Monday, 8 September 2014 at 12:36:29 UTC, Marco Leise wrote:
> 
> > I think the template bloat argument is invalid as __LINE__ and friends are passed as template arguments to allow write and writef type logging.

You are right, this benefit of classes doesn't apply here.

> > Anyway I will try to make them free standing
> 
> The biggest problem I have currently with this that you, or at least I, can not override the free standing function.
> 
> void log(L)(ref L logger) if(isLogger!L) { ... } will match always
> and if I create void log(L)(ref L logger) if(isMySpecialLogger!L)
> { ... }
> both match and thats a nogo

Ok, no matter what the outcome is, I'll see if I can write a simple file logger that I can use in RAII struct dtors (where neither allocations nor throwing seem to be an issue) and that has a fallback to writing to stderr. I wrote earlier that I would want a fallback logger if writing via the network fails or the disk is full, but maybe this logic can be implemented inside a logger implementation. I haven't actually tried your API yet!

-- 
Marco

On Monday, 8 September 2014 at 14:49:06 UTC, Marco Leise wrote:
>
> Ok, no matter what the outcome is, I'll see if I can write a
> simple file logger that I can use in RAII struct dtors (where
> neither allocations nor throwing seem to be an issue) and that
> has a fallback to writing to stderr. I wrote earlier that I
> would want a fallback logger if writing via the network fails
> or the disk is full, but maybe this logic can be implemented
> inside a logger implementation. I haven't actually tried your
> API yet!

That should be a no-brainer just have a look at FileLogger and start from there

On Wednesday, 3 September 2014 at 03:05:42 UTC, Kevin Lamonte wrote:
> On Tuesday, 2 September 2014 at 14:53:17 UTC, Dicebot wrote:
>> This is exactly what I call theoretical speculations. Please provide specific list like this:
>>
>> 1) some method signature needs to be changed
>
> I propose the following API changes (+ changes on default implementation):
>
>     protected LogEntry beginLogMsg(string file, int line, string funcName,
>         string prettyFuncName, string moduleName, LogLevel logLevel,
>         Tid threadId, SysTime timestamp, Logger logger)
>         @trusted
>     {
>         static if (isLoggingActive())
>         {
>             return LogEntry(file, line, funcName, prettyFuncName,
>                 moduleName, logLevel, threadId, timestamp, null, logger);
>         }
>     }
>
>     /** Logs a part of the log message. */
>     protected void logMsgPart(MsgRange msgAppender, const(char)[] msg)
>     {
>         static if (isLoggingActive())
>         {
>             msgAppender.put(msg);
>         }
>     }
>
>     /** Signals that the message has been written and no more calls to
>     $(D logMsgPart) follow. */
>     protected void finishLogMsg(ref LogEntry entry, MsgRange msgAppender)
>     {
>         static if (isLoggingActive())
>         {
>             entry.msg = msgAppender.data;
>             this.writeLogMsg(entry);
>         }
>     }
>
> ...with the corresponding changes to logImpl/logImplf to create msgAppender as a local function variable, and the elimination of header and msgAppender as Logger class variables.
>
> The benefit to this change is that Logger (including stdlog) becomes thread-safe, as well as any subclass of Logger that only implements writeLogMsg().

The only problem with that change is that it will always require a buffer. FileLogger currently doesn't require a buffer and is already thread-safe.

stdlog will not be thread-safe by default by this change only syncing the writeLogMsg function will.

On Monday, 8 September 2014 at 11:06:44 UTC, Robert burner Schadek wrote:
>> First part partially addressed - missing @nogc @nothrow logger implementation out of the box. Considering this request does not go very well with current language implementation it may be ignored for now but official stance about it must be clearly documented.
>
> at least for logf nothrow will not work because of a wrong formatting string or args. log can not be nothrow because custom toString for structs and class are allowed.
>
> nogc is not possible because of custom toString
>
> that won't fix, but for default types it is nogc.

It should be possible to provide custom implementation that throws ad Error for those cases (and thus fits the requirements) and `toString` has sink-based overload. Are there any reason why it doesn't help?

On Monday, 8 September 2014 at 22:54:36 UTC, Dicebot wrote:
>> nogc is not possible because of custom toString
>>
>> that won't fix, but for default types it is nogc.
>
> It should be possible to provide custom implementation that throws ad Error for those cases (and thus fits the requirements) and `toString` has sink-based overload. Are there any reason why it doesn't help?

catching an Exception from formattedWrite just to throw an Error and thus allowing nothrow is just silly IMO.
sink-based overloads are nice but we don't write the toString methods of the user and so can not be sure that they are nogc.

On Saturday, 30 August 2014 at 02:18:30 UTC, Dicebot wrote:
> I have likely missed several points but overall it seem pretty clear to me that all requests / concerns have not been addressed and this proposal is not yet ready for another round of review.

I made the stdlog creating thread-safe and on stack. I think that was the last point that was mentioned.

>
> Also x-post from GitHub PR of my personal nitpick:
>
> "... have noticed that all logging functions have file/line/function data as template parameters. This will create insane symbol bloat. While I can understand desire to use some nicer variadic argument API providing at least one log function that it simplistic but moves all compile-time data to run-time default argument is absolutely necessary for me to consider this viable."

I do not consider that a problem. The benefit is much higher than the cost of the bigger binary. This has already been a long conversation on some previous thread.