use template programming instead of macros for logging

[Erroneous1]

fastcgipp/include/fastcgi++/log.hpp:94:69: error: declaration of ‘lock’ shadows a previous local [-Werror=shadow]
...
fastcgipp/src/sockets.cpp:547:25: note: in expansion of macro ‘FAIL_LOG’

This is just one example. Using the lock_guard called lock shadows any other variable called lock. You could use a variadic template instead and acheive the same results. Just use , instead of <<. I can submit a PR if you want.

[eddic]

Hmmmm. As it sits now I'm not convinced of any reason why shadowing in this situation is actually a problem. Isn't shadowing fully permitted in the C++ standard? Why do you feel shadowing should be disallowed?

[Erroneous1]

First I'm a fan of removing macros when they can be replaced by C++ standards. I guess my issue isn't so much with the shadowing, more of the use of macros. Shadowing may be allowed, but imho if it can be avoided it should be.

Here's a sample function I'm proposing:

template<class ... Args>
inline void infoLog(Args ... args)
{
std::lock_guard<std::mutex> lock{::Fastcgipp::Logging::mutex};
::Fastcgipp::Logging::header(::Fastcgipp::Logging::INFO);
int dummy[] = { (*::Fastcgipp::Logging::logstream << args, 0)... };
*::Fastcgipp::Logging::logstream << std::endl;
}
This performs all args operations on logstream and then does an endl.

[aggsol]

Shadowing can cause really hard to find bugs. If comes from Macro expansion then it is even harder. I agree with @Erroneous1 suggested changes.

[eddic]

Hmmmm. You know. I must admit that I really do prefer the variadic template solution to my macros. That is the way it should have been done to begin with. That being said, I'm not willing to break all the code that's been written so far by changing the interface. This isn't an alpha stage; it's a beta stage and that means sticking with these kind of design choices even when they are shown to be less ideologically sound. Certainly in the next major version change, your solution will be preferred.

Beyond all of that, can anyone present me with a hypothetical situation where this shadowing would lead to a bug? Just for discussions sake.

[Erroneous1]

I can't think of a particular situation where shadowing would cause a bug when used in a matter like this. Generally though if you are expecting a variable named X to be of type Y and you declare X as Z that could be an issue. Or if you declare X as Y twice and expect X outside of the scope you declared it the second time to still be what it was inside the scope.

A quick and easy mitigation would be to use a unique name for the lock_guard like fastcgipp_fail_log_lock. That would give you time to consider how you would like to replace the macros.

Another possible implementation would be to make a class that contains a lock_guard and a reference/pointer to an ostream that passes ostream& operator<< to that reference. FAIL_LOG's std::exit is the only reason why you might not want to do that.