9
\$\begingroup\$
#include <iostream>

const bool DEBUG = false;

template <typename ... Args>
void debug(Args ... args);

template <typename ... Args>
void _debug(Args ... args) {
    std::cout << "[" << __FILE__ << ":" << __LINE__ << "] ";
    (std::cout << ... << args) << std::endl;
}

#define debug(args...) if (DEBUG) _debug(args);

Suppose you are running ACM or you are somewhere without a well-crafted development environment. You have written some code and wondering what is going wrong.

Goal here is to find out what cause the problem as quickly as possible by placing tracing prints and turn them off all together by preventing code generation for them. So you can submit your code with DEBUG = false and it will not cause any performance issues done by calling debug(...).

\$\endgroup\$
  • \$\begingroup\$ Does that print file name and line number of the caller? \$\endgroup\$ – Martin R Oct 17 at 8:39
  • \$\begingroup\$ @MartinR GCC - gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html MSVC - docs.microsoft.com/en-us/cpp/preprocessor/… \$\endgroup\$ – outoftime Oct 17 at 8:41
  • 3
    \$\begingroup\$ I know what those macros are. But did you try it? – I did (with Xcode/clang on macOS) and the output was the file/number of where the template is define in the debug.hpp file, not where the macro is called in main.cpp. \$\endgroup\$ – Martin R Oct 17 at 8:45
  • 3
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Vogel612 Oct 17 at 11:12
12
\$\begingroup\$

Although it's a short function/macro, there are a number of problems here:

  • __FILE__ and __LINE__ are expanded in the function definition, rather than at the call site. We need an interface that passes those in, so that the macro is

    #define debug(args...) if (DEBUG) _debug(__FILE__, __LINE__, args);
    
  • That's a non-standard variadic macro expansion. The standard way is to write the parameter as ... and expand it using __VA_ARGS__.

  • Identifiers in global scope beginning with underscore are reserved for the implementation. Use a namespace instead (e.g. debugging::debug).

  • Debug information should go to std::clog, not std::cout.

  • The macro doesn't play nicely in if/else statements - use the do...while(0) idiom to make it statement-like.

  • Conventional style is to use all-caps for macros (to indicate their dangers) and nothing else. We have this exactly backwards here, with debug and DEBUG.


Improved version

#include <iostream>

namespace debugging
{
#ifdef ENABLE_DEBUG
    constexpr bool debug = true;
#else
    constexpr bool debug = false;
#endif

    template <typename... Args>
    void print(const char* file, int line, Args... args) {
        (std::clog << "[" << file << ":" << line << "] "
                  << ... << args) << std::endl;
    }
}

#define DEBUG(...)                                              \
    do {                                                        \
        if (debugging::debug)                                   \
            debugging::print(__FILE__, __LINE__, __VA_ARGS__);  \
    } while (0)

And a quick test (that demonstrates that the arguments are evaluated only when we're debugging):

int main()
{
    DEBUG("Started main");
    int status = 1;
    DEBUG("Leaving main, status=", status=0);
    return status;
}

Addendum

We don't need the debug constant - just change the definition of the macro according to whether or not we're debugging:

#ifndef ENABLE_DEBUG
#define DEBUG(...)  ((void)0)
#else
#include <iostream>

namespace debugging
{
    template<typename... Args>
    void print(const char* file, int line, Args... args) {
        (std::clog << "[" << file << ":" << line << "] "
                  << ... << args) << std::endl;
    }
}

#define DEBUG(...)  debugging::print(__FILE__, __LINE__, __VA_ARGS__)
#endif
\$\endgroup\$
  • \$\begingroup\$ Why constexpr bool instead of const bool ? \$\endgroup\$ – outoftime Oct 17 at 11:13
  • \$\begingroup\$ Fix error: binary expression in operand of fold-expression pls. Adding additional parentess around init expr will do the trick. \$\endgroup\$ – outoftime Oct 17 at 11:24
  • \$\begingroup\$ I can't remember why I went for constexpr there - perhaps just defensiveness. I also considered if constexpr because we might be using variables that we initialize only if debugging - we don't want to trigger warnings in those cases. \$\endgroup\$ – Toby Speight Oct 17 at 11:53
  • 3
    \$\begingroup\$ Note: In C++20, prefer std::source_location over __FILE__ and __LINE__. Prior to C++20, consider std::experimental::source_location. In fact, with source_location, you could even consider making debug(...) a (possibly [[gnu::always_inline]]) function rather than a macro, possibly allowing lambda-wrapped parameters to allow lazy evaluation. \$\endgroup\$ – Justin Oct 18 at 0:10
  • 2
    \$\begingroup\$ Why bother with defining a constexpr variable when you already have a preprocessor flag? Just use that for the conditional... I don't see an advantage in the indirection. \$\endgroup\$ – Cody Gray Oct 18 at 3:12
4
\$\begingroup\$

Each of those insertion operators can be interleaved with other calls in a multi-threaded environment.

Consider building the string in memory first, then making a single call to std::cout. This approach will also give better behavior with stream buffering disabled as is typical with std::cerr.

\$\endgroup\$
  • 1
    \$\begingroup\$ What if program does some IO and I want to tie my debug info to it? \$\endgroup\$ – outoftime Oct 18 at 2:49
  • \$\begingroup\$ I'm not too clear on what you're asking, but I think you'd like to know how to send the debug output somewhere else. You could always pass in the stream object as a parameter. \$\endgroup\$ – eric Oct 18 at 20:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.