Project

General

Profile

Feature #5966

Add a move constructor to the loggers

Added by Gianluca Petrillo almost 7 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
04/18/2014
Due date:
% Done:

100%

Estimated time:
8.00 h
Spent time:
Duration:

Description

Loggers are in general non-copiable, with a few exceptions. One of them is LogDebug.
Copiable loggers are, indeed, copiable, but the resulting copy is unusable, possibly by the concept that the underlying resource must still be unique and therefore it's not copied.

LOG_DEBUG is a macro which can return either a LogDebug or a Neverlogger_.
Trying to save its result implies a copy (on SLF6 at least!):

auto log = LOG_DEBUG("Category") << "First line.\n";
log << "Second line.";

will throw an exception on the second instruction.
In this situation, I don't really want a copy, I just want to move the whole thing from a temporary object to a local, named one.
The implementation of a move constructor and assignment for both CopiableLogger_ and MaybeLogger_ should allow the code above (or a similar one) to produce a working logger. NeverLogger_ is not an issue since it does not own any resource.
I am using art and C++11.


Related issues

Related to messagefacility - Support #7421: Remove CopyableLogger_ and replace DEFINE_LOGGER macro with class templateClosed12/03/2014

History

#1 Updated by Christopher Green almost 7 years ago

  • Status changed from New to Accepted
  • Target version set to 1.10.00
  • Estimated time set to 8.00 h

We believe this should not be too tricky to do with C++2011, but with this code there is always the risk of unforeseen complications, hence the conservative time estimate.

#2 Updated by Christopher Green almost 7 years ago

  • Target version changed from 1.10.00 to 1.13.00

#3 Updated by Kyle Knoepfel over 6 years ago

  • Assignee set to Kyle Knoepfel

#4 Updated by Kyle Knoepfel over 6 years ago

  • Status changed from Accepted to Feedback

We would like to discuss this requested feature with you in person. There are unforeseen complications here that have wide-reaching consequences.

#5 Updated by Kyle Knoepfel over 6 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 0 to 100

Gianluca, Chris G. and I spoke about this yesterday, and in the end we were able to implement this feature with narrower changes than the previous update in this issue suggested there would be. This issue required resolving two concerns:

  • implementing the move construction itself, and
  • ensuring that undesired loggable messages are indeed suppressed.

As discussed in Issue #7421, CopyableLogger_ has been removed and move construction has been enabled for MaybeLogger_, thus resolving the first concern. Regarding the second, an explanation of the LOG_DEBUG macro is in order.

For run-time determined debug logging, the LOG_DEBUG macro definition is such that the following code:

auto log = LOG_DEBUG("debug") << "foo" << object_needing_a_lot_of_work_to_stream_out;
log << "bar";

is semantically equivalent to (suppressing the additional __FILE__ and __LINE__ arguments to mf::LogDebug)

auto log = !debugEnabled ? mf::LogDebug("debug") : mf::LogDebug("debug") << "foo" << object_needing_a_lot_of_work_to_stream_out;
log << "bar";

In the event that debugEnabled is false (true), the result of the second (third) expression is returned. For the case when the second expression is returned, all streaming work is suppressed in that line. Note, however, that once the log << "bar"; statement is reached, all logging resumes unconditionally. The resolution to this problem is related to a probable bug that was discovered in the definition of the mf::LogDebug constructor.

The mf::LogDebug constructor was created so that a message object was always created. Even if no messages were provided by the user, the message object would still log the preamble and epilogue:

%MSG-d debug:  MFTest {Timestamp} pro-event ELdestinationTester.cc:151

%MSG

We suspect this was a bug. To address this issue, the mf::LogDebug constructor now creates a message object only when debugEnabled is true. In this case, the log<<"bar"; statement will still execute, but the boolean check ( similar to if ( messageObject ) ) inside of the definition of the << operator will prevent any further logging from taking place since the object doesn't exist. This resolves the second concern.

Implemented with fbead7e1d893c2009c97fa6328b901463e93b4d7.

#6 Updated by Christopher Green about 6 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF