Forráskód Böngészése

AK+Kernel+Userland: Enable some more compiletime format string checks

This enables format string checks for three more functions:
- String::formatted()
- Builder::appendff()
- KBufferBuilder::appendff()
AnotherTest 4 éve
szülő
commit
7c2754c3a6

+ 2 - 2
AK/String.h

@@ -270,9 +270,9 @@ public:
     static String vformatted(StringView fmtstr, TypeErasedFormatParams);
 
     template<typename... Parameters>
-    static String formatted(StringView fmtstr, const Parameters&... parameters)
+    static String formatted(CheckedFormatString<Parameters...>&& fmtstr, const Parameters&... parameters)
     {
-        return vformatted(fmtstr, VariadicFormatParams { parameters... });
+        return vformatted(fmtstr.view(), VariadicFormatParams { parameters... });
     }
 
     template<typename T>

+ 2 - 2
AK/StringBuilder.h

@@ -52,9 +52,9 @@ public:
     void append_escaped_for_json(const StringView&);
 
     template<typename... Parameters>
-    void appendff(StringView fmtstr, const Parameters&... parameters)
+    void appendff(CheckedFormatString<Parameters...>&& fmtstr, const Parameters&... parameters)
     {
-        vformat(*this, fmtstr, VariadicFormatParams { parameters... });
+        vformat(*this, fmtstr.view(), VariadicFormatParams { parameters... });
     }
 
     String build() const;

+ 12 - 8
AK/Tests/TestFormat.cpp

@@ -58,8 +58,10 @@ TEST_CASE(reorder_format_arguments)
 {
     EXPECT_EQ(String::formatted("{1}{0}", "a", "b"), "ba");
     EXPECT_EQ(String::formatted("{0}{1}", "a", "b"), "ab");
-    EXPECT_EQ(String::formatted("{0}{0}{0}", "a", "b"), "aaa");
-    EXPECT_EQ(String::formatted("{1}{}{0}", "a", "b", "c"), "baa");
+    // Compiletime check bypass: ignoring a passed argument.
+    EXPECT_EQ(String::formatted(StringView { "{0}{0}{0}" }, "a", "b"), "aaa");
+    // Compiletime check bypass: ignoring a passed argument.
+    EXPECT_EQ(String::formatted(StringView { "{1}{}{0}" }, "a", "b", "c"), "baa");
 }
 
 TEST_CASE(escape_braces)
@@ -123,16 +125,18 @@ TEST_CASE(zero_pad)
 
 TEST_CASE(replacement_field)
 {
-    EXPECT_EQ(String::formatted("{:*>{1}}", 13, static_cast<size_t>(10)), "********13");
-    EXPECT_EQ(String::formatted("{:*<{1}}", 7, 4), "7***");
-    EXPECT_EQ(String::formatted("{:{2}}", -5, 8, 16), "              -5");
-    EXPECT_EQ(String::formatted("{{{:*^{1}}}}", 1, 3), "{*1*}");
-    EXPECT_EQ(String::formatted("{:0{}}", 1, 3), "001");
+    // FIXME: Compiletime check bypass: cannot parse '}}' correctly.
+    EXPECT_EQ(String::formatted(StringView { "{:*>{1}}" }, 13, static_cast<size_t>(10)), "********13");
+    EXPECT_EQ(String::formatted(StringView { "{:*<{1}}" }, 7, 4), "7***");
+    EXPECT_EQ(String::formatted(StringView { "{:{2}}" }, -5, 8, 16), "              -5");
+    EXPECT_EQ(String::formatted(StringView { "{{{:*^{1}}}}" }, 1, 3), "{*1*}");
+    EXPECT_EQ(String::formatted(StringView { "{:0{}}" }, 1, 3), "001");
 }
 
 TEST_CASE(replacement_field_regression)
 {
-    EXPECT_EQ(String::formatted("{:{}}", "", static_cast<unsigned long>(6)), "      ");
+    // FIXME: Compiletime check bypass: cannot parse '}}' correctly.
+    EXPECT_EQ(String::formatted(StringView { "{:{}}" }, "", static_cast<unsigned long>(6)), "      ");
 }
 
 TEST_CASE(complex_string_specifiers)

+ 2 - 2
Kernel/KBufferBuilder.h

@@ -49,11 +49,11 @@ public:
     void append_bytes(ReadonlyBytes);
 
     template<typename... Parameters>
-    void appendff(StringView fmtstr, const Parameters&... parameters)
+    void appendff(CheckedFormatString<Parameters...>&& fmtstr, const Parameters&... parameters)
     {
         // FIXME: This is really not the way to go about it, but vformat expects a
         //        StringBuilder. Why does this class exist anyways?
-        append(String::formatted(fmtstr, parameters...));
+        append(String::formatted(fmtstr.view(), parameters...));
     }
 
     bool flush();

+ 5 - 2
Userland/Applications/Calculator/Keypad.cpp

@@ -164,8 +164,11 @@ String Keypad::to_string() const
         builder.append("-");
     builder.appendff("{}", m_int_value);
 
-    if (m_frac_length > 0)
-        builder.appendff(".{:0{}}", m_frac_value, m_frac_length);
+    if (m_frac_length > 0) {
+        // FIXME: This disables the compiletime format string check since we can't parse '}}' here correctly.
+        //        remove the 'StringView { }' when that's fixed.
+        builder.appendff(StringView { ".{:0{}}" }, m_frac_value, m_frac_length);
+    }
 
     return builder.to_string();
 }

+ 1 - 1
Userland/Applications/CrashReporter/main.cpp

@@ -60,7 +60,7 @@ static TitleAndText build_backtrace(const CoreDump::Reader& coredump, const ELF:
 
     StringBuilder builder;
 
-    auto prepend_metadata = [&](auto& key, auto& fmt) {
+    auto prepend_metadata = [&](auto& key, StringView fmt) {
         auto maybe_value = coredump.metadata().get(key);
         if (!maybe_value.has_value() || maybe_value.value().is_empty())
             return;