Poco::Logger can not output __FILE__ and __LINE__

[siren186]

Describe the bug Poco::Logger can not output __FILE__ and __LINE__

To Reproduce

// Here is the problem, It's very strange in my Visual Studio 2019 (with Qt5.15.2)
// I don't know why, The name should be "int", But in fact, name == "long"
std::string name = typeid(__LINE__).name();

poco_information(logger(), "hello"); // Error !!! Log has no __FILE__ and __LINE__
logger().information("hello", __FILE__, __LINE__); // Error !!! Log has no __FILE__ and __LINE__
logger().information("hello", __FILE__, (int)__LINE__); // OK

If the type of __LINE__ is int, Poco::Logger will call this method:

void information(const std::string& msg, const char* file, int line);

But if the type of __LINE__ is long, Poco::Logger will call this method:

template <typename T, typename... Args>
void information(const std::string& fmt, T arg1, Args&&... args)
{
if (information())
	logAlways(Poco::format(fmt, arg1, std::forward<Args>(args)...), Message::PRIO_INFORMATION);
}

Then it will lost __FILE__ and __LINE__.

Please add relevant environment information:

• Window 10
• poco-1.13.3
• Visual Studio 2019

[andrewauclair]

Are you running into this issue with /ZI in Debug? https://github.com/pocoproject/poco/issues/2913#issuecomment-586155527

[siren186]

Are you running into this issue with /ZI in Debug? #2913 (comment)

Yes, you are right ! What about add some methods like

void information(const std::string& msg, const char* file, int line);
void information(const std::string& msg, const char* file, long int line); // new method to add

to fix the problem ?

[andrewauclair]

That would be a breaking change to any existing calls to information that use those types. The only real option that comes to mind is to use static_cast<int>(__LINE__) instead of just __LINE__ in the logger macros.

[bas524]

Hi! What do you think about replacing method

void information(const std::string& msg, const char* file, int line);

with

template<typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
void information(const std::string& msg, const char* file, T line);

? IMO this isn't breaking change

[aleks-f]

@bas524 that looks like it should work

[bas524]

@siren186, can you help me to set correct compiler options? I want to verify my changes in godbolt, but I can't get described behavour when __LINE__ is long link with sample code

[andrewauclair]

IMO this isn't breaking change

Opinions aside, this is a breaking change. Using a template doesn't change the output. The same thing would happen with just void information(const std::string& msg, const char* file, long line). See this godbolt for a comparison: https://godbolt.org/z/v9hGsjd8s.

Any calls to the parameter pack version of information such as information("%s:%d", "127.0.0.1", 5000L) would be broken.

[bas524]

@andrewauclair , I think that I found another good way to resolve the problem without templates I fix type of __LINE__ with decltype and use this type for declaration

namespace Poco {

using LineNumber = decltype(__LINE__);

}

and then

void log(const std::string& text, Message::Priority prio, const char* file, LineNumber line);

[andrewauclair]

That would cause the same issue. Although it would only cause the issue for when the type of __LINE__ is long. I'm also not sure that would actually work if Poco was compiled without ZI but the application was.

The ultimate (but sadly still breaking) change would be to separate the log functions that takes file and line from the other log functions. I've wanted to add some optional C++20 logging with std::source_location for a while as well.

[siren186]

@siren186, can you help me to set correct compiler options? I want to verify my changes in godbolt, but I can't get described behavour when __LINE__ is long link with sample code

It compiled with errors.

@andrewauclair , I think that I found another good way to resolve the problem without templates I fix type of __LINE__ with decltype and use this type for declaration

namespace Poco {

using LineNumber = decltype(__LINE__);

}

and then

void log(const std::string& text, Message::Priority prio, const char* file, LineNumber line);

It works well !!!

[matejk]

@andrewauclair , I think that I found another good way to resolve the problem without templates I fix type of __LINE__ with decltype and use this type for declaration

namespace Poco {

using LineNumber = decltype(__LINE__);

}

and then

void log(const std::string& text, Message::Priority prio, const char* file, LineNumber line);

IMO this is the most elegant solution because the type for the line number is derived from the compiler/platform.

[andrewauclair]

Here's what I think would be a good compromise. A breaking change is required to properly account for long __LINE__ and the current logging functions that log file and line number interfere with the variadic template versions of the functions. It would be good to separate these into different functions. To make sure the breaking change is clear, we can mark the existing log function with file and line as deleted. Code then can be migrated to the new logWithLocation and the deleted version can be removed after a release or two. This would allow using the variadic template form with those arguments.

As a bonus, we can define a version of logWithVersion that uses the new std::source_location if the header is available. The actual check for this include would be a lot messier than this, but this is just an example. This can technically be done with the variadic template form as well using deduction guides to allow the proper calling of the function.

Godbolt for this code: https://godbolt.org/z/rM1T3d5qa

#include <iostream>
#include <string>
#include <format>

#if __has_include(<source_location>)
#include <source_location>
#endif

struct Logger {
    // delete the existing log function with file and line to make the breaking change obvious
    void log(const std::string& msg, const char* file, int line) = delete;

    // add a new logging function with a different name to prevent confusion with the variadic template version
    void logWithLocation(const std::string& msg, const char* file, decltype(__LINE__) line) {
        std::cout << file << ":" << line << ": " << msg << '\n';
    }

    // new log function with location using the C++20 std::source_location for information
#if __has_include(<source_location>)
    void logWithLocation(const std::string& msg, std::source_location location = std::source_location::current()) {
        std::cout << location.file_name() << '('
            << location.line() << ':'
            << location.column() << ") `"
            << location.function_name() << ": " << msg << '\n';
    }
#endif

    template<typename T, typename... Args>
    void log (const std::string& format, T t, Args... args) {
        std::cout << std::vformat(format, std::make_format_args(t, args...)) << '\n';
    }
};

int main()
{
    Logger logger;

    logger.logWithLocation("failed here", __FILE__, __LINE__);
    logger.logWithLocation("failed again");
    logger.log("{}:{}", "127.0.0.1", 5000L);
}

/app/example.cpp:38: failed here
/app/example.cpp(39:27) `int main(): failed again
127.0.0.1:5000