[GitHub] [pulsar] BewareMyPower commented on a change in pull request #11668: [Issue 11632][C++] Turning on more compiler warnings, and enforcing warnings as errors

[GitBox <gi...@apache.org>]

BewareMyPower commented on a change in pull request #11668:
URL: https://github.com/apache/pulsar/pull/11668#discussion_r694899772



##########
File path: pulsar-client-cpp/lib/LogUtils.h
##########
@@ -35,7 +35,7 @@ namespace pulsar {
 #endif
 
 #define DECLARE_LOG_OBJECT()                                                                     \
-    static pulsar::Logger* logger() {                                                            \
+    inline pulsar::Logger* logger() {                                                            \

Review comment:
       We should not change `static` here to `inline`. Though `inline` can also avoid multiple definitions error, it will cause that a random definition is chosen in all translation units. But for each translation unit, we need a dependent definition of `logger()` because the `__FILE__` macro is different.
   
   This change has made logs less readable, for example, see logs in https://github.com/apache/pulsar/issues/11760. All log lines' file is `ReaderTest` because it has chosen `logger()` definition from `ReaderTest.cc` for all translation units.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org