Avoid vsnprintf when possible

templates/src/diagnostic.c.erb

@@ -12,6 +12,7 @@
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 #define PM_DIAGNOSTIC_ID_MAX <%= errors.length + warnings.length %>
 
@@ -534,11 +535,16 @@ pm_diagnostic_list_append(pm_arena_t *arena, pm_list_t *list, uint32_t start, ui
  */
 void
 pm_diagnostic_list_append_format(pm_arena_t *arena, pm_list_t *list, uint32_t start, uint32_t length, pm_diagnostic_id_t diag_id, ...) {
+    /* Format the message into a buffer on the stack, which nearly every message
+     * fits in. The result is the length of the whole message whether or not it
+     * fit, so a single pass both formats the message and sizes it. */
+    char stack[256];
+
     va_list arguments;
     va_start(arguments, diag_id);
 
     const char *format = pm_diagnostic_id_message(diag_id);
-    int result = vsnprintf(NULL, 0, format, arguments);
+    int result = vsnprintf(stack, sizeof(stack), format, arguments);
     va_end(arguments);
 
     if (result < 0) {
@@ -550,9 +556,15 @@ pm_diagnostic_list_append_format(pm_arena_t *arena, pm_list_t *list, uint32_t st
     size_t message_length = (size_t) (result + 1);
     char *message = (char *) pm_arena_alloc(arena, message_length, 1);
 
-    va_start(arguments, diag_id);
-    vsnprintf(message, message_length, format, arguments);
-    va_end(arguments);
+    if (message_length <= sizeof(stack)) {
+        memcpy(message, stack, message_length);
+    } else {
+        /* The message was cut off on the stack, so format it again into an
+         * allocation of the right size. */
+        va_start(arguments, diag_id);
+        vsnprintf(message, message_length, format, arguments);
+        va_end(arguments);
+    }
 
     *diagnostic = (pm_diagnostic_t) {
         .location = { .start = start, .length = length },

test/prism/errors_test.rb

@@ -83,6 +83,21 @@ def test_regexp_encoding_option_mismatch_error
       assert_empty result.errors
     end
 
+    def test_long_formatted_messages
+      # Messages are formatted into a fixed-size buffer first, so check lengths
+      # on either side of its size to cover both the copy and the reformat.
+      template = "unterminated heredoc; can't find string \"%s\" anywhere before EOF"
+
+      (254..258).each do |length|
+        identifier = "E" * (length - template.length + 2)
+        expected = format(template, identifier)
+        assert_equal length, expected.bytesize
+
+        messages = Prism.parse("<<#{identifier}\n").errors.map(&:message)
+        assert_includes messages, expected
+      end
+    end
+
     def test_incomplete_def_closing_loc
       statement = Prism.parse_statement("def f; 123")
       assert_nil(statement.end_keyword)