Skip to content

Removed many global imports from std.experimental.logger.core #5092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
WalterBright merged 1 commit into from
Feb 10, 2017
Merged

Removed many global imports from std.experimental.logger.core #5092

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 50 additions & 10 deletions std/experimental/logger/core.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
///
module std.experimental.logger.core;

import std.array;
import std.stdio;
import std.conv;
import std.datetime;
import std.string;
import std.range;
import std.range.primitives;
import std.traits;
Copy link
Member

@PetarKirov PetarKirov Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use selective imports?

Copy link
Contributor Author

@JackStouffer JackStouffer Feb 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's really no benefit to top-level selective imports at the moment. DMD still scans the entire imported file; all selective imports do is change visibility rules. This is something that can and should change (SDC does the right thing), and when it does, I will gladly fix this.

Plus, selective top level imports isn't the current style in the rest of Phobos.

Copy link
Member

@PetarKirov PetarKirov Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest benefit for me personally is that they document explicitly what is being imported. This makes them easier to convert to convert to local imports and to whatever DIP1005 brings to the table (language change or something along the lines of dlang/druntime#1756).

Plus, selective top level imports isn't the current style in the rest of Phobos.

Where we are at the moment is not interesting to talk about. What matters is where we want to be and how to get there.

Copy link
Contributor

@joakim-noah joakim-noah Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a benefit right now: the compiler has to search all the top-level modules and any publicly imported modules they reference to see where a symbol is invoked from and check for name collisions. When you specify the module a symbol comes from, that search is skipped.

The real issue is that we have held off on using module-scope selective imports until we're sure the symbol leak bugs are fixed and downstream code has adapted. Now that 2.073 has shipped, maybe @MartinNowak will OK their use?

import std.exception;
import std.concurrency;
import std.format;
import core.atomic;
import core.sync.mutex : Mutex;

import std.experimental.logger.filelogger;
Expand Down Expand Up @@ -175,6 +167,8 @@ The fractional second part is in milliseconds and is always 3 digits.
void systimeToISOString(OutputRange)(OutputRange o, const ref SysTime time)
if (isOutputRange!(OutputRange,string))
{
import std.format : formattedWrite;

const auto dt = cast(DateTime)time;
const auto fsec = time.fracSecs.total!"msecs";

Expand Down Expand Up @@ -667,7 +661,7 @@ private void formatString(A...)(MsgRange oRange, A args)

foreach (arg; args)
{
std.format.formattedWrite(oRange, "%s", arg);
formattedWrite(oRange, "%s", arg);
}
}

Expand Down Expand Up @@ -716,6 +710,9 @@ flexibility.
*/
abstract class Logger
{
import std.array : appender, Appender;
import std.concurrency : thisTid, Tid;

/** LogEntry is a aggregation combining all information associated
with a log message. This aggregation will be passed to the method
writeLogMsg.
Expand Down Expand Up @@ -1044,6 +1041,8 @@ abstract class Logger
static if (isLoggingActiveAt!ll && ll >= moduleLogLevel!moduleName)
synchronized (mutex)
{
import std.format : formattedWrite;

if (isLoggingEnabled(ll, this.logLevel_, globalLogLevel,
condition))
{
Expand Down Expand Up @@ -1090,6 +1089,8 @@ abstract class Logger
static if (isLoggingActiveAt!ll && ll >= moduleLogLevel!moduleName)
synchronized (mutex)
{
import std.format : formattedWrite;

if (isLoggingEnabled(ll, this.logLevel_, globalLogLevel))
{
this.beginLogMsg(file, line, funcName, prettyFuncName,
Expand Down Expand Up @@ -1441,6 +1442,8 @@ abstract class Logger
{
static if (isLoggingActive) synchronized (mutex)
{
import std.format : formattedWrite;

if (isLoggingEnabled(ll, this.logLevel_, globalLogLevel, condition))
{
this.beginLogMsg(file, line, funcName, prettyFuncName,
Expand Down Expand Up @@ -1487,6 +1490,8 @@ abstract class Logger
{
static if (isLoggingActive) synchronized (mutex)
{
import std.format : formattedWrite;

if (isLoggingEnabled(ll, this.logLevel_, globalLogLevel))
{
this.beginLogMsg(file, line, funcName, prettyFuncName,
Expand Down Expand Up @@ -1534,6 +1539,8 @@ abstract class Logger
{
static if (isLoggingActive) synchronized (mutex)
{
import std.format : formattedWrite;

if (isLoggingEnabled(this.logLevel_, this.logLevel_, globalLogLevel,
condition))
{
Expand Down Expand Up @@ -1578,6 +1585,8 @@ abstract class Logger
{
static if (isLoggingActive) synchronized (mutex)
{
import std.format : formattedWrite;

if (isLoggingEnabled(this.logLevel_, this.logLevel_,
globalLogLevel))
{
Expand Down Expand Up @@ -1615,6 +1624,9 @@ private shared LogLevel stdLoggerGlobalLogLevel = LogLevel.all;
*/
private @property Logger defaultSharedLoggerImpl() @trusted
{
import std.conv : emplace;
import std.stdio : stderr;

static __gshared ubyte[__traits(classInstanceSize, FileLogger)] _buffer;

synchronized (stdSharedLoggerMutex)
Expand Down Expand Up @@ -1654,6 +1666,7 @@ if (sharedLog !is myLogger)
{
static auto trustedLoad(ref shared Logger logger) @trusted
{
import core.atomic : atomicLoad, MemoryOrder;
return atomicLoad!(MemoryOrder.acq)(logger);
}

Expand All @@ -1672,6 +1685,7 @@ if (sharedLog !is myLogger)
/// Ditto
@property void sharedLog(Logger logger) @trusted
{
import core.atomic : atomicStore, MemoryOrder;
atomicStore!(MemoryOrder.rel)(stdSharedLogger, cast(shared) logger);
}

Expand Down Expand Up @@ -1743,6 +1757,8 @@ private Logger stdLoggerDefaultThreadLogger;
*/
private @property Logger stdThreadLocalLogImpl() @trusted
{
import std.conv : emplace;

static ubyte[__traits(classInstanceSize, StdForwardLogger)] _buffer;

auto buffer = cast(ubyte[]) _buffer;
Expand Down Expand Up @@ -1925,6 +1941,10 @@ version(unittest) private void testFuncNames(Logger logger) @safe

@safe unittest
{
import std.conv : to;
import std.exception : assertThrown, assertNotThrown;
import std.format : format;

auto l = new TestLogger(LogLevel.all);
string msg = "Hello Logger World";
l.log(msg);
Expand Down Expand Up @@ -2064,6 +2084,8 @@ version(unittest) private void testFuncNames(Logger logger) @safe
unittest // default logger
{
import std.file : deleteme, exists, remove;
import std.stdio : File;
import std.string : indexOf;

string filename = deleteme ~ __FUNCTION__ ~ ".tempLogFile";
FileLogger l = new FileLogger(filename);
Expand Down Expand Up @@ -2102,6 +2124,8 @@ unittest // default logger
unittest
{
import std.file : deleteme, remove;
import std.stdio : File;
import std.string : indexOf;

string filename = deleteme ~ __FUNCTION__ ~ ".tempLogFile";
auto oldunspecificLogger = sharedLog;
Expand Down Expand Up @@ -2134,6 +2158,8 @@ unittest

@safe unittest
{
import std.conv : to;

auto tl = new TestLogger(LogLevel.all);
int l = __LINE__;
tl.info("a");
Expand All @@ -2150,6 +2176,10 @@ unittest
// testing possible log conditions
@safe unittest
{
import std.conv : to;
import std.string : indexOf;
import std.format : format;

auto oldunspecificLogger = sharedLog;

auto mem = new TestLogger;
Expand Down Expand Up @@ -2393,6 +2423,10 @@ unittest
// more testing
@safe unittest
{
import std.conv : to;
import std.format : format;
import std.string : indexOf;

auto oldunspecificLogger = sharedLog;

auto mem = new TestLogger;
Expand Down Expand Up @@ -2877,6 +2911,8 @@ unittest
// Issue #5
@safe unittest
{
import std.string : indexOf;

auto oldunspecificLogger = sharedLog;

scope(exit)
Expand All @@ -2896,6 +2932,8 @@ unittest
@safe unittest
{
import std.experimental.logger.multilogger : MultiLogger;
import std.string : indexOf;

stdThreadLocalLog.logLevel = LogLevel.all;

auto oldunspecificLogger = sharedLog;
Expand Down Expand Up @@ -2946,12 +2984,14 @@ unittest
// Workaround for atomics not allowed in @safe code
private auto trustedLoad(T)(ref shared T value) @trusted
{
import core.atomic : atomicLoad, MemoryOrder;
return atomicLoad!(MemoryOrder.acq)(value);
}

// ditto
private void trustedStore(T)(ref shared T dst, ref T src) @trusted
{
import core.atomic : atomicStore, MemoryOrder;
atomicStore!(MemoryOrder.rel)(dst, src);
}

Expand Down