0

I was trying the logger code from this link, but it gives me error. How to implement a good debug/logging feature in a project

#ifndef _LOGGER_HPP_
#define _LOGGER_HPP_

#include <iostream>
#include <sstream>

/* consider adding boost thread id since we'll want to know whose writting and
 * won't want to repeat it for every single call */

/* consider adding policy class to allow users to redirect logging to specific
 * files via the command line
 */

enum loglevel_e
    {logERROR, logWARNING, logINFO, logDEBUG, logDEBUG1, logDEBUG2, logDEBUG3, logDEBUG4};

class logIt
{
public:
    logIt(loglevel_e _loglevel = logERROR) {
        _buffer << _loglevel << " :" 
            << std::string(
                _loglevel > logDEBUG 
                ? (_loglevel - logDEBUG) * 4 
                : 1
                , ' ');
    }

    template <typename T>
    logIt & operator<<(T const & value)
    {
        _buffer << value;
        return *this;
    }

    ~logIt()
    {
        _buffer << std::endl;
        // This is atomic according to the POSIX standard
        // http://www.gnu.org/s/libc/manual/html_node/Streams-and-Threads.html
        std::cerr << _buffer.str();
    }

private:
    std::ostringstream _buffer;
};

extern loglevel_e loglevel;

#define log(level) \
if (level > loglevel) ; \
else logIt(level)

#endif

More precisely, this #define is giving errors:

#define log(level) \
if (level > loglevel) ; \
else logIt(level)

The errors are Syntax error: if and Syntax error: else

But later, I noticed that if I move #include "logger.hpp" from main.h to main.cpp, the problem disappeared. Though 'main.h' is included many times in different places, it does contain '#pragma once'.

Any idea?

Community
  • 1
  • 1
Boyang
  • 2,520
  • 5
  • 31
  • 49
  • That's text replacement. Look at the PP output. – chris Nov 05 '13 at 17:26
  • You are missing a semicolon after `logit(level)` – Erbureth Nov 05 '13 at 17:26
  • As @chris has indicated, the errors occur because log(level) is just replaced by the if-statement in the definition, and that is leading to invalid code that the compiler is choking on. Could you add an example of a place in your code where you are using this and getting the errors, with a few lines around it so we can see the context? – CompuChip Nov 05 '13 at 17:28
  • @Erbureth The usage is `logIt(level) << "debug msg";` logIt(level) is a call to the constructor. So I think semicolon shouldn't be there? – Boyang Nov 05 '13 at 17:29
  • @CompuChip It wouldn't compile – Boyang Nov 05 '13 at 17:29
  • what does `logIt` return? – Erbureth Nov 05 '13 at 17:30
  • Please always show the exact error meassage. – Vlad from Moscow Nov 05 '13 at 17:32
  • Your code compiled for me without any problems (Using g++-4.8.2) – Erbureth Nov 05 '13 at 17:33
  • You're using a [reserved identifier](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). – chris Nov 05 '13 at 17:34
  • Also the `;` after the `if (level > loglevel)` is wrong, it should be an empty block `{}` instead. – πάντα ῥεῖ Nov 05 '13 at 17:49
  • Hi all, I realized that previously, I put `#include "logger.hpp"` in `main.h`. And many files include `main.h`. Now I moved `#include "logger.hpp"` to 'main.cpp', and it solved the problem... But why? I do have `#pragma once` in `main.h` – Boyang Nov 05 '13 at 17:57
  • Further point: The declaration of `logIt & operator<<(T const & value)` should be `logIt & operator<<(T const & value) const` to work the way you want to use it from your macro, because you have a temporary instance of `logIt` which is inherently a `const` object (make `_buffer` `mutable` then). – πάντα ῥεῖ Nov 05 '13 at 19:11
  • No idea why moving the `#include` statement to main.cpp seems to fix your problem, but I'm pretty sure it won't work the way you're showing it here, no matter how directly or indirectly included from whatever .cpp file. – πάντα ῥεῖ Nov 05 '13 at 19:16

3 Answers3

2

If loglevel is known at compile time you can do the following:

template <bool>
struct LogSystem
{
    template <class T>
    LogSystem& operator << (const T &)
    {
        //ignore the input
        return (*this);
    }
};

template <>
struct LogSystem <true>
{
    template <class T>
    LogSystem& operator << (const T & v)
    {
        cout << v;
        return (*this);
    }
};

template <bool B>
LogSystem<B>    getLog()
{
    return LogSystem<B>();
}

#define log(level) getLog< (level <= loglevel) >()

if loglevel is not known at compile time:

class iLogSystem
{
public:
    virtual iLogSystem& operator << (const int &)
    {
        //empty
        return (*this);
    }
    virtual iLogSystem& operator << (const custom_type &);
    {
        return (*this);
    }
    //make functions for logging all the types you want
};

class LogSystem : public iLogSystem
{
public:
    virtual iLogSystem& operator << (const int & v)
    {
        cout << v;
        return (*this);
    }
    virtual iLogSystem& operator << (const custom_type &  q);
    {
        cout << q.toString();
        return (*this);
    }
    //make functions for logging all the types you want
};
iLogSystem& getLog(const bool l)
{
    static LogSystem actual_log;
    static iLogSystem empty_log;
    if(l)
        return &actual_log;
    return &empty_log;
}

#define log(level) getLog( level <= loglevel )
Raxvan
  • 6,257
  • 2
  • 25
  • 46
2

Can #define preprocessor directive contain if and else?

Yes.

Regarding your problem: preprocessor is dumb as a rock and performs only simple text subsitution. It is not a function, it is not a language construct, it is simple, dumb text subsitution. As a a result, this:

#define log(level) \
    if (level > loglevel) ; \
    else logIt(level)

...

log(logINFO) << "foo " << "bar " << "baz";

Turns into this:

if (logINFO > loglevel); // << here's your problem.
else 
    logIt(logInfo)
 << "foo " << "bar " << "baz"; 

Your problem is;. Here, semicolon indicates end of c++ if statement, so when compiler encounters else afterwards, it doesn't know what to do with it.

I noticed that if I move #include "logger.hpp" from main.h to main.cpp, the problem disappeared

C++ has "logarithm" function. Which is called log. If your other files use logarithm function, things will get very interesting, because it'll be replaced by your if/else logging code everywhere.

For example, if there's inlined logarithm code somewhere in a header, it'll turn into nonsense if you include your logger header first. For example log(6.0) + 1 will turn into log (if (6.0 > logLevel); else logIt(6.0)) + 1, which is not a valid C++ statement.

SigTerm
  • 26,089
  • 6
  • 66
  • 115
  • Besides the point with the `log()` function, you might also mention what I said in my last comment on the question: The `logIt::operator<<()` declaration doesn't fit to be applied to a `const logIt`. – πάντα ῥεῖ Nov 05 '13 at 19:34
2

Any time you want to define a macro that expands to a statement, if the definition contains any compound statements (including if/else), you should wrap the definition in do ... while (0). The enclosed code will still execute exactly once, and it can be used in any context that requires a statement.

That's the only way I know of to avoid syntax errors when the macro is used within an if/else statement, due to the use of semicolons.

So rather that this:

#define log(level) \
    if ((level) > loglevel) ; \
    else logIt(level)

you can use this:

#define log(level) \
    do { \
        if ((level) > loglevel) ; \
        else logIt(level) \
    } while (0)

I've added parentheses around references to the macro's level parameter, to avoid any possible operator precedence problems. Note also the lack of a semicolon at the end; the semicolon will be supplied by the caller.

On the other hand, an if/else can often be replaced by the conditional (ternary) ?: operator:

#define log(level) \
    ((level) > loglevel ? 0 : logIt(level))

which allows log(level) to be used anywhere an expression can be used; that includes statement context if you add a semicolon. You might want to replace 0 by something of the type returned by logIt; if logIt is a void function, you might want:

#define log(level) \
    ((level) > loglevel ? (void)0 : logIt(level))

This all assumes that a macro is the right tool for the job. It's likely that a template (as suggested by this answer) or an inline function will do the job better and with less potential for confusion.

Community
  • 1
  • 1
Keith Thompson
  • 254,901
  • 44
  • 429
  • 631