On Tuesday, 29 July 2014 at 06:09:25 UTC, Andrei Alexandrescu
wrote:
My vote is a qualified "yes" contingent upon fixes that I'll
give detail on below. In the current form my vote is "no"
seeing as the module makes a number of unforced tactical
errors. Overall I think the goods are there, and are easy to
put in acceptable form.
Here's my list:
1. Minimal logging level must be selected statically in
addition to the current dynamic selection. Static settings
preclude dynamic settings. This is not negotiable.
I'm not sure how you except log(LogLevel.xxxx, "Hello world") to
be disabled at compile time if LogLevel.xxxx is a runtime value?
Or do I misunderstood you?
you can choose to disable name based logging like trace("Hello
trace") at CT with the current release
2. All logging code must be rigorously eliminated if below the
static logging level. More precisely, there must be a front-end
optimization that eliminates all code dedicated to a "lazy"
variable that's not used by a generic function. This would be a
fantastic redeeming of the "lazy" keyword, which has been of
marginal utility until now. The challenge here is cooperating
with someone on the compiler team to make sure that front-end
improvement gets implemented, and writing unit tests that make
sure there's no regression later. This is not negotiable.
If you disabled one on the names logging functions at CT other
prototypes will be used that have no lazy in it. You said that
empty functions with lazy parameter are not optimized away. So
there are no empty functions with lazy parameter if you disable
these functions. As soon as the compiler can kill empty functions
with lazy arguments this branching can be removed without any
user code adjustment.
3. The suffix notations must be replaced with overloads. The
only acceptable suffix is "f" for formatting. Everything else
must be achieved via overloads with the help of template
constraints. Robert's answer http://goo.gl/FehDVh suggests he
didn't consider using template constraints. We can't let that
slip become a feature of the library for millenia to come.
The overloads must be:
// just log stuff
log(T...)(lazy T stuff) if (!is(T[0] : const LogLevel));
// log with format
logf(S, T...)(lazy S fmt, lazy T stuff) if (isSomeString!Str);
// log conditional with format
logf(S, T...)(lazy bool cond, lazy S fmt, lazy T stuff) if
(isSomeString!Str);
These three overloads should be repeated for all logging
functions (info, trace etc). The functions don't evaluate their
arguments if the respective log level is disabled.
The following functions will NOT be repeated for all logging
functions:
// just log stuff at some level
log(T...)(LogLevel lvl, lazy T stuff) if (!is(T[0] : const
LogLevel));
// log with format
logf(S, T...)(LogLevel lvl, lazy S fmt, lazy T stuff) if
(isSomeString!Str);
// log conditional with format
logf(S, T...)(LogLevel lvl, lazy bool cond, lazy S fmt, lazy T
stuff) if (isSomeString!Str);
These overloads always evaluate their first argument eagerly to
determine the required logging level. Depending on it they may
or may not evaluate their other arguments.
This is not negotiable.
Overloads are implemented in the current version. They behave as
you described.
4. Replace defaultLogger with theLog. "Logger" is a word, but
one that means "lumberjack" so it doesn't have the appropriate
semantics. The use is generally acceptable as a nice play on
words and as a disambiguator between the verb "to log" and the
noun "log". When we actually want to talk about the current log
in an application, we should, however, call it "the log". This
is negotiable.
I really don't care how a global Logger instance is called.
Anyone else has an opinion on this? Otherwise Andrei wins.
5. I was hoping for a resolution on throttling. However, now I
realize that conditional logging plus throttling functions that
return true once in a while should work acceptably well.
Higher-order functions taking lambdas that log would also be a
nice possibility. So... no request here.
Creating a std.logger.conditions module is on my todo,
std.logger.(stderr,stdout) will be cut because of being to noisy.
I'm thinking of
* anyN
* anyNmillisec
* firstN
* ...
6. I'm still hoping for RefCounted as the storage for the class
backend. I realize users can do their own management but log
objects are unlikely to contain cycles and other liabilities
for reference counting, and at some point if we want to use
reference counting where appropriate we got to start somewhere
with a few solid precedents. This is negotiable, but I plan to
fight for it.
IMO something is wrong in the users code if the GC working on
Logger instances is slowing the code done. The Logger instances
properly stay around for the length of the programs execution. If
you create Logger in a tight loop RC will properly slow you down
as well.
4) Any additional comments for author.
Don't let any of the above discourage you. This is great work
and is already one foot in. Let's get this done and done. Don't
forget - it's all about Deutsche Gründlichkeit!
Andrei
Hope this brings you closer to a "yes"
Robert
