Bug 2668 - NS_LOG can not be used in templates
NS_LOG can not be used in templates
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: core
ns-3-dev
All All
: P1 enhancement
Assigned To: Peter Barnes
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-08 18:55 EST by Tommaso Pecorella
Modified: 2017-06-06 05:20 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tommaso Pecorella 2017-03-08 18:55:18 EST
In changeset 12738:d523e255e050 Stefano added a "NDQI_LOG" macro.
The new macro is needed to allow the use of logging facilities in templatized classes.

Although the solution is working as expected, the trick can not be extended without a macro proliferation.

It would be better to have a more general system to use NS_LOG from templates.
Comment 1 Peter Barnes 2017-03-08 20:20:07 EST
I haven't stared at the Queue<> design enough, so this may be gibberish:

Isn't this the point of base classes, and QueueItem in particular? to give a minimal type for things like the things being Queued?  So instead of 

template <typename Item>
class Queue : public QueueBase
{
    bool Enqueue (Ptr<Item> item);
...

You do

class Queue : public QueueBase
{
    bool Enqueue (Ptr<QueueItem> item);
...
Comment 2 Peter Barnes 2017-03-08 20:30:59 EST
Failing that, it seems the macro issue here has to do with emitting log messages from one compilation unit (which includes net-device-queue-interface.h, for example), but which really belong to the log component defined in another compilation unit, namely "NetDeviceQueueInterface" defined in net-device-queue-interface.cc.  

Perhaps we need 

static LogComponent & LogComponent::GetComponent (const std::string log);

Then we can define logging macros like this:
#define NS_LOG(log, level, msg) ...
#define NS_LOG_DEBUG(msg)  NS_LOG(g_log, ns3::LOG_DEBUG, msg)
#define NS_LOG_DEBUG(log, msg)  NS_LOG(LogComponent::GetComponent(log), ns3::LOG_DEBUG, msg)

The second line will use the log component defined in the current compilation unit; the third line will use the designated log component.

To define NS_LOG_DEBUG with either two or three args we would have to use the variadic macros tricks here: http://stackoverflow.com/questions/16683146

or define a new set of macros
#define NS_LOG_DEBUG_COMP(log, msg) NS_LOG(LogComponent::GetComponent(log), ns3::LOG_DEBUG, msg)
Comment 3 Stefano Avallone 2017-03-09 11:50:37 EST
(In reply to Peter Barnes from comment #2)
> Failing that, it seems the macro issue here has to do with emitting log
> messages from one compilation unit (which includes
> net-device-queue-interface.h, for example), but which really belong to the
> log component defined in another compilation unit, namely
> "NetDeviceQueueInterface" defined in net-device-queue-interface.cc.  
> 
> Perhaps we need 
> 
> static LogComponent & LogComponent::GetComponent (const std::string log);
> 
> Then we can define logging macros like this:
> #define NS_LOG(log, level, msg) ...
> #define NS_LOG_DEBUG(msg)  NS_LOG(g_log, ns3::LOG_DEBUG, msg)
> #define NS_LOG_DEBUG(log, msg)  NS_LOG(LogComponent::GetComponent(log),
> ns3::LOG_DEBUG, msg)
> 
> The second line will use the log component defined in the current
> compilation unit; the third line will use the designated log component.
> 
> To define NS_LOG_DEBUG with either two or three args we would have to use
> the variadic macros tricks here: http://stackoverflow.com/questions/16683146
> 
> or define a new set of macros
> #define NS_LOG_DEBUG_COMP(log, msg) NS_LOG(LogComponent::GetComponent(log),
> ns3::LOG_DEBUG, msg)

This is a very interesting solution, thanks!

I will try to experiment with this approach as soon as I can.
Comment 4 Stefano Avallone 2017-03-27 11:58:59 EDT
I elaborated Peter's suggestion a bit and implemented two approaches (only one is working atm...). Basically, we can reuse the classic NS_LOG_* macros if template classes define a LogComponent g_log member variable. The first (working) approach defines such member variables as non-static:

https://codereview.appspot.com/320790043/

The second (non working) approach is an experiment I made to define the g_log member variables as static:

https://codereview.appspot.com/319590043/

The issue here is the static initialization order (these member variables must be initialized to the log component defined in other compilation units). As a solution, I introduced an auxiliary object that creates a LogComponent the first time it is invoked and returns the created component the next times. However, all programs crash in a strange way: the LogComponent::IsEnabled method is called before any object of type LogComponent (or LogComponentRegistration) is constructed.

Any opinion? Thanks!
Comment 5 Tommaso Pecorella 2017-03-27 18:01:42 EDT
Hi,

personally, I like enough option 1.
I'll leave to Peter the final decision.

Cheers,

T.

PS. we need also to document in the manual the slightly different use of the logging facilities for template / non-template classes.


(In reply to Stefano Avallone from comment #4)
> I elaborated Peter's suggestion a bit and implemented two approaches (only
> one is working atm...). Basically, we can reuse the classic NS_LOG_* macros
> if template classes define a LogComponent g_log member variable. The first
> (working) approach defines such member variables as non-static:
> 
> https://codereview.appspot.com/320790043/
> 
> The second (non working) approach is an experiment I made to define the
> g_log member variables as static:
> 
> https://codereview.appspot.com/319590043/
> 
> The issue here is the static initialization order (these member variables
> must be initialized to the log component defined in other compilation
> units). As a solution, I introduced an auxiliary object that creates a
> LogComponent the first time it is invoked and returns the created component
> the next times. However, all programs crash in a strange way: the
> LogComponent::IsEnabled method is called before any object of type
> LogComponent (or LogComponentRegistration) is constructed.
> 
> Any opinion? Thanks!
Comment 6 Stefano Avallone 2017-05-31 03:51:54 EDT
What about pushing option 1 so that it gets included in ns-3.27?
Comment 7 Tom Henderson 2017-06-02 10:39:21 EDT
(In reply to Stefano Avallone from comment #6)
> What about pushing option 1 so that it gets included in ns-3.27?

Can you implement changes that Peter suggests (macros, and documentation update) in the next few days?  If so, we can include.
Comment 8 Stefano Avallone 2017-06-02 12:41:31 EDT
I had a look at Peter's comments (thanks!), which I think are easy to address. I will provide a new patch by Monday.
Comment 9 Stefano Avallone 2017-06-06 05:20:35 EDT
Pushed option 1 with changeset 12922:d5736db31887, thanks.