Logger should use makeString instead of String::format
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Aug 2017 02:59:18 +0000 (02:59 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Aug 2017 02:59:18 +0000 (02:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176035

Reviewed by Jer Noble.

Source/WebCore/PAL:

* pal/Logger.h:
(PAL::LogArgument::toString):
(PAL::Logger::logAlways):
(PAL::Logger::error):
(PAL::Logger::warning):
(PAL::Logger::notice):
(PAL::Logger::info):
(PAL::Logger::debug):
(PAL::Logger::MethodAndPointer::MethodAndPointer):
(PAL::Logger::log):
(PAL::LogArgument<Logger::MethodAndPointer>::toString):

Tools:

* TestWebKitAPI/Tests/WebCore/Logging.cpp:
(TestWebKitAPI::TEST_F): Update test.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221288 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/PAL/ChangeLog
Source/WebCore/PAL/pal/Logger.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/Logging.cpp

index e114c28..f20102e 100644 (file)
@@ -1,3 +1,22 @@
+2017-08-28  Eric Carlson  <eric.carlson@apple.com>
+
+        Logger should use makeString instead of String::format
+        https://bugs.webkit.org/show_bug.cgi?id=176035
+
+        Reviewed by Jer Noble.
+
+        * pal/Logger.h:
+        (PAL::LogArgument::toString):
+        (PAL::Logger::logAlways):
+        (PAL::Logger::error):
+        (PAL::Logger::warning):
+        (PAL::Logger::notice):
+        (PAL::Logger::info):
+        (PAL::Logger::debug):
+        (PAL::Logger::MethodAndPointer::MethodAndPointer):
+        (PAL::Logger::log):
+        (PAL::LogArgument<Logger::MethodAndPointer>::toString):
+
 2017-08-28  Andy Estes  <aestes@apple.com>
 
         [Cocoa] Upstream CFNetwork-related WebKitSystemInterface functions
index afc1227..da37d43 100644 (file)
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #pragma once
 
 #include <wtf/Assertions.h>
+#include <wtf/HexNumber.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
+#include <wtf/text/StringBuilder.h>
 #include <wtf/text/WTFString.h>
 
 namespace PAL {
 
+template<typename T>
+struct LogArgument {
+    template<typename U = T> static typename std::enable_if<std::is_same<U, int>::value, String>::type toString(int argument) { return String::number(argument); }
+    template<typename U = T> static typename std::enable_if<std::is_same<U, float>::value, String>::type toString(float argument) { return String::number(argument); }
+    template<typename U = T> static typename std::enable_if<std::is_same<U, double>::value, String>::type toString(double argument) { return String::number(argument); }
+    template<typename U = T> static typename std::enable_if<std::is_same<U, String>::value, String>::type toString(String argument) { return argument; }
+    template<typename U = T> static typename std::enable_if<std::is_same<U, const char*>::value, String>::type toString(const char* argument) { return String(argument); }
+    template<size_t length> static String toString(const char (&argument)[length]) { return String(argument); }
+};
+
 class Logger : public RefCounted<Logger> {
     WTF_MAKE_NONCOPYABLE(Logger);
 public:
@@ -42,64 +54,63 @@ public:
     }
 
     template<typename... Arguments>
-    inline void logAlways(WTFLogChannel& channel, const char* format, const Arguments&... arguments) const WTF_ATTRIBUTE_PRINTF(3, 0)
+    inline void logAlways(WTFLogChannel& channel, const Arguments&... arguments)
     {
 #if RELEASE_LOG_DISABLED
         // "Standard" WebCore logging goes to stderr, which is captured in layout test output and can generally be a problem
         //  on some systems, so don't allow it.
         UNUSED_PARAM(channel);
-        UNUSED_PARAM(format);
 #else
         if (!willLog(channel, WTFLogLevelAlways))
             return;
 
-        log(channel, format, arguments...);
+        log(channel, arguments...);
 #endif
     }
 
     template<typename... Arguments>
-    inline void error(WTFLogChannel& channel, const char* format, const Arguments&... arguments) const WTF_ATTRIBUTE_PRINTF(3, 0)
+    inline void error(WTFLogChannel& channel, const Arguments&... arguments)
     {
         if (!willLog(channel, WTFLogLevelError))
             return;
 
-        log(channel, format, arguments...);
+        log(channel, arguments...);
     }
 
     template<typename... Arguments>
-    inline void warning(WTFLogChannel& channel, const char* format, const Arguments&... arguments) const WTF_ATTRIBUTE_PRINTF(3, 0)
+    inline void warning(WTFLogChannel& channel, const Arguments&... arguments)
     {
         if (!willLog(channel, WTFLogLevelWarning))
             return;
 
-        log(channel, format, arguments...);
+        log(channel, arguments...);
     }
 
     template<typename... Arguments>
-    inline void notice(WTFLogChannel& channel, const char* format, const Arguments&... arguments) const WTF_ATTRIBUTE_PRINTF(3, 0)
+    inline void notice(WTFLogChannel& channel, const Arguments&... arguments)
     {
         if (!willLog(channel, WTFLogLevelNotice))
             return;
 
-        log(channel, format, arguments...);
+        log(channel, arguments...);
     }
 
     template<typename... Arguments>
-    inline void info(WTFLogChannel& channel, const char* format, const Arguments&... arguments) const WTF_ATTRIBUTE_PRINTF(3, 0)
+    inline void info(WTFLogChannel& channel, const Arguments&... arguments)
     {
         if (!willLog(channel, WTFLogLevelInfo))
             return;
 
-        log(channel, format, arguments...);
+        log(channel, arguments...);
     }
 
     template<typename... Arguments>
-    inline void debug(WTFLogChannel& channel, const char* format, const Arguments&... arguments) const WTF_ATTRIBUTE_PRINTF(3, 0)
+    inline void debug(WTFLogChannel& channel, const Arguments&... arguments)
     {
         if (!willLog(channel, WTFLogLevelDebug))
             return;
 
-        log(channel, format, arguments...);
+        log(channel, arguments...);
     }
 
     inline bool willLog(WTFLogChannel& channel, WTFLogLevel level) const
@@ -121,35 +132,48 @@ public:
             m_enabled = enabled;
     }
 
+    struct MethodAndPointer {
+        MethodAndPointer(const char* methodName, void* objectPtr)
+            : methodName(methodName)
+            , objectPtr(reinterpret_cast<uintptr_t>(objectPtr))
+        {
+        }
+
+        const char* methodName;
+        uintptr_t objectPtr;
+    };
+
 private:
     Logger(const void* owner) { m_owner = owner; }
 
-    static inline void log(WTFLogChannel& channel, const char* format, ...)
+    template<typename... Argument>
+    static inline void log(WTFLogChannel& channel, const Argument&... arguments)
     {
-        va_list arguments;
-        va_start(arguments, format);
-
-#if COMPILER(GCC_OR_CLANG)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wformat-nonliteral"
-#endif
-        String string = String::formatWithArguments(format, arguments);
-#if COMPILER(GCC_OR_CLANG)
-#pragma GCC diagnostic pop
-#endif
+        String string = makeString(LogArgument<Argument>::toString(arguments)...);
 
 #if RELEASE_LOG_DISABLED
         WTFLog(&channel, "%s", string.utf8().data());
 #else
         os_log(channel.osLogChannel, "%{public}s", string.utf8().data());
 #endif
-
-        va_end(arguments);
     }
 
     const void* m_owner;
     bool m_enabled { true };
 };
 
+template <>
+struct LogArgument<Logger::MethodAndPointer> {
+    static String toString(const Logger::MethodAndPointer& value)
+    {
+        StringBuilder builder;
+        builder.append(value.methodName);
+        builder.appendLiteral("(0x");
+        appendUnsigned64AsHex(value.objectPtr, builder);
+        builder.appendLiteral(") ");
+        return builder.toString();
+    }
+};
+
 } // namespace PAL
 
index e704a01..eda9fd9 100644 (file)
@@ -1,3 +1,13 @@
+2017-08-28  Eric Carlson  <eric.carlson@apple.com>
+
+        Logger should use makeString instead of String::format
+        https://bugs.webkit.org/show_bug.cgi?id=176035
+
+        Reviewed by Jer Noble.
+
+        * TestWebKitAPI/Tests/WebCore/Logging.cpp:
+        (TestWebKitAPI::TEST_F): Update test.
+
 2017-08-28  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [GStreamer] The glvideoflip GStreamer element isn't available. Video mirroring and rotation functionalities are thus disabled.
index ab7bb8f..f42735d 100644 (file)
@@ -267,9 +267,8 @@ TEST_F(LoggingTest, Logger)
     EXPECT_TRUE(logger->enabled());
 
     WTFSetLogChannelLevel(&TestChannel1, WTFLogLevelError);
-    logger->error(TestChannel1, "%s %s", "What,", "ridden on a horse?");
-    EXPECT_TRUE(output().contains("What, ridden on a horse?", false));
-
+    logger->error(TestChannel1, "You're using coconuts!");
+    EXPECT_TRUE(output().contains("You're using coconuts!", false));
     logger->warning(TestChannel1, "You're using coconuts!");
     EXPECT_EQ(0u, output().length());
     logger->notice(TestChannel1, "You're using coconuts!");
@@ -279,6 +278,15 @@ TEST_F(LoggingTest, Logger)
     logger->debug(TestChannel1, "You're using coconuts!");
     EXPECT_EQ(0u, output().length());
 
+    logger->error(TestChannel1, Logger::MethodAndPointer("LoggingTest::Logger", this) , ": test output");
+    EXPECT_TRUE(output().contains("LoggingTest::Logger(", false));
+
+    logger->error(TestChannel1, "What is ", 1, " + " , 12.5F, "?");
+    EXPECT_TRUE(output().contains("What is 1 + 12.5?", false));
+
+    logger->error(TestChannel1, "What, ", "ridden on a horse?");
+    EXPECT_TRUE(output().contains("What, ridden on a horse?", false));
+
     logger->setEnabled(this, false);
     EXPECT_FALSE(logger->enabled());
     logger->error(TestChannel1, "You've got two empty halves of coconuts");
@@ -286,11 +294,11 @@ TEST_F(LoggingTest, Logger)
 
     logger->setEnabled(this, true);
     EXPECT_TRUE(logger->enabled());
-    logger->error(TestChannel1, "%s %s", "You've got two empty halves of", "coconuts!");
-    EXPECT_TRUE(output().contains("You've got two empty halves of coconuts!", false));
+    logger->error(TestChannel1, "You've got ", 2, " empty halves of ", "coconuts!");
+    EXPECT_TRUE(output().contains("You've got 2 empty halves of coconuts!", false));
 
     WTFSetLogChannelLevel(&TestChannel1, WTFLogLevelError);
-    logger->logAlways(TestChannel1, "%s", "I shall taunt you a second time!");
+    logger->logAlways(TestChannel1, "I shall taunt you a second time!");
     EXPECT_TRUE(output().contains("I shall taunt you a second time!", false));
 
     logger->setEnabled(this, false);