Fix 1477: Add (ly:expect-warning msg args) to suppress expected warnings

If a file contains (ly:expect-warning ....), the corresponding warning
string will be added to a list of expected warnings. If the corresponding
warning (or erro) is triggered, it will not be printed to stderr, but the
string will be removed from the list. So, each ly:expect-warning, suppresses
exactly one occurrence of the warning. To suppress a warning multiple times,
call ly:expect-warning multiple times.

After one file is processed, the list of expected warnings is checked.
If it is not empty, it means that an expected warning was not triggered,
which might be a bug. So we print out a warning message about this fact.

This allows the regtests to check proper warning messages, without
polluting the console output with those warning messages.
All warnings that are actually printed in the regtests are a bug.

For translated error message, there are two approaches: If the warning
is created from scheme, simply use
       (ly:expect-warning (_ "msg with ~a") "args")
If the message is triggered from C++, the translated string is in printf
syntax, so we need to translate that and then convert it into a format
string for Scheme. This can be done with the new function
ly:translate-cpp-warning-scheme instead of _:
       (ly:expect-warning (ly:translate-cpp-warning-scheme "msg with %s") "args")

This patch does not yet adapt all regtests that are supposed to print warnings
to this new approach. This will follow in a separate patch.

diff --git a/flower/include/warn.hh b/flower/include/warn.hh
index 3b66f65b7bf48bbeb149a1c3dfed35265ec7ad66..f17bef2804b734c395a01a0e78ebc11e9bb3e65e 100644 (file)
--- a/flower/include/warn.hh
+++ b/flower/include/warn.hh
@@ -61,4 +61,8 @@ bool is_loglevel (int level);
 void set_loglevel (int level);
 void set_loglevel (string level);
 
+void expect_warning (string msg);
+void check_expected_warnings ();
+
+
 #endif /* WARN_HH */

diff --git a/flower/warn.cc b/flower/warn.cc
index a6e1fa5f90b27c1c3416877e78eb80a8ad4ebc49..af16f20bebe0dd347253e775493fe9b61bd82ba6 100644 (file)
--- a/flower/warn.cc
+++ b/flower/warn.cc
@@ -22,6 +22,7 @@
 #include <cstdlib>
 #include <cstdio>
 
+#include "std-vector.hh"
 #include "international.hh"
 
 using namespace std;
@@ -91,6 +92,51 @@ set_loglevel (string level)
     }
 }
 
+/**
+ * Register a warning string to be expected and the output suppressed.
+ * If the warning is encountered, it will be removed from the list of
+ * expected warnings again.
+ */
+vector<string> expected_warnings;
+void expect_warning (string msg)
+{
+  expected_warnings.push_back (msg);
+}
+
+void check_expected_warnings ()
+{
+  if (expected_warnings.size () > 0) 
+    {
+      /* Some expected warning was not triggered, so print out a warning. */
+      string msg = _f ("%d expected warning(s) not encountered: ", 
+                       expected_warnings.size ());
+      for (vsize i = 0; i< expected_warnings.size (); i++)
+          msg += "\n        " + expected_warnings[i];
+      
+      warning (msg);
+    }
+  expected_warnings.clear ();
+}
+
+bool is_expected (string s)
+{
+  bool expected = false;
+  for (vsize i = 0; i< expected_warnings.size (); i++)
+    {
+      // Compare the msg with the suppressed string; If the beginning matches,
+      // i.e. the msg can have additional content AFTER the full (exact)
+      // suppressed message, suppress the warning.
+      // This is needed for the Input class, where the message contains
+      // the input file contents after the real message.
+      if (s.compare (0, expected_warnings[i].size (), expected_warnings[i]) == 0 ) {
+        expected = true;
+        expected_warnings.erase (expected_warnings.begin () + i);
+        break;
+      }
+    }
+  return expected;
+}
+
 
 /**
  * Helper functions: print_message_part (no newline prepended)
@@ -142,22 +188,35 @@ error (string s, string location)
 void
 programming_error (string s, string location)
 {
-  print_message (LOG_ERROR, location, _f ("programming error: %s", s) + "\n");
-  print_message (LOG_ERROR, location, _ ("continuing, cross fingers") + "\n");
+  if (is_expected (s)) {
+    print_message (LOG_DEBUG, location, _f ("suppressed programming error: %s", s) + "\n");
+  } else {
+    print_message (LOG_ERROR, location, _f ("programming error: %s", s) + "\n");
+    print_message (LOG_ERROR, location, _ ("continuing, cross fingers") + "\n");
+  }
 }
 
 /* Display a non-fatal error message, don't exit.  */
 void
 non_fatal_error (string s, string location)
 {
-  print_message (LOG_ERROR, location, _f ("error: %s", s) + "\n");
+  if (is_expected (s))
+    print_message (LOG_DEBUG, location, _f ("suppressed error: %s", s) + "\n");
+  else {
+    print_message (LOG_ERROR, location, _f ("error: %s", s) + "\n");
+  }
 }
 
 /* Display a warning message. */
 void
 warning (string s, string location)
 {
-  print_message (LOG_WARN, location, _f ("warning: %s", s) + "\n");
+  if (is_expected (s))
+    print_message (LOG_DEBUG, location, _f ("suppressed warning: %s", s) + "\n");
+  else {
+    // TODO: Add warning-as-error check here
+    print_message (LOG_WARN, location, _f ("warning: %s", s) + "\n");
+  }
 }
 
 /* Display a success message.  */

diff --git a/lily/warn-scheme.cc b/lily/warn-scheme.cc
index 69fba1b8b7d94b14fed096a679e337e37e9db935..2f7a1af611f0f442c7577b589f010130a3dab27e 100644 (file)
--- a/lily/warn-scheme.cc
+++ b/lily/warn-scheme.cc
@@ -23,6 +23,7 @@
 #include "lily-guile.hh"
 #include "program-option.hh"
 #include "version.hh"
+#include "international.hh"
 #include "warn.hh"
 
 /*
@@ -136,3 +137,73 @@ LY_DEFINE (ly_warning_located, "ly:warning-located",
 
   return SCM_UNSPECIFIED;
 }
+
+LY_DEFINE (ly_expect_warning, "ly:expect-warning",
+           1, 0, 1, (SCM str, SCM rest),
+           "A Scheme callable function to register a warning to be expected"
+           " and subsequently suppressed.  If the warning is not encountered,"
+           " a warning about the missing warning will be shown. The message"
+           " should be translated with @code{(_ ...)} and changing parameters"
+           " given after the format string.")
+{
+  LY_ASSERT_TYPE (scm_is_string, str, 1);
+  str = scm_simple_format (SCM_BOOL_F, str, rest);
+  expect_warning (ly_scm2string (str));
+  return SCM_UNSPECIFIED;
+}
+
+LY_DEFINE (ly_check_expected_warnings, "ly:check-expected-warnings",
+           0, 0, 0, (),
+           "Check whether all expected warnings have really been triggered.")
+{
+  check_expected_warnings ();
+  return SCM_UNSPECIFIED;
+}
+
+LY_DEFINE (ly_translate_cpp_warning_scheme, "ly:translate-cpp-warning-scheme",
+           1, 0, 0, (SCM str),
+           "Translates a string in C++ printf format and modifies it to use"
+           " it for scheme formatting.")
+{
+  LY_ASSERT_TYPE (scm_is_string, str, 1);
+  string s = _ (ly_scm2string (str).c_str ());
+  
+  /* Now replace all printf placeholders by scheme placeholders (~a).
+   * Guile's format syntax is pretty similar to C's printf, only with
+   * a tilde as the placeholder instead of a percent sign.
+   * There is no easy way to replace all ~ -> ~~, %% -> %, % -> ~,
+   * so simply walk through each character.
+   */
+//   size_t pos = 0;
+  const char *pos = s.c_str ();
+  string result = "";
+  while (*pos != '\0')
+    {
+      // In some cases (%%, %s) we need to do a lookahead. As the C string is
+      // always \0-terminated the next char is never beyond the end of the
+      // memory!
+      switch (*pos) {
+        case '~':
+          result += "~~";
+          break;
+        case '%':
+          if (*(pos+1) == '%') {
+            result += "%";
+            // Skip the second '%'
+            pos++;
+          } else if (*(pos+1) == 's' || *(pos+1) == 'd') {
+            // %s in C++ corresponds to ~a; ~s would add quotes!
+            // ~d is only supported by ice-9, use ~a instead
+            result += "~a";
+            // Skip the following 's'
+            pos++;
+          } else
+            result += "~";
+          break;
+        default:
+          result += *pos;
+      }
+      pos++;
+    }
+  return ly_string2scm (result);
+}

diff --git a/scm/lily.scm b/scm/lily.scm
index feec950c45e782717dcfae1b1d2fd1c84ede3111..c971758f0d2b7db2d22816196587e72536384493 100644 (file)
--- a/scm/lily.scm
+++ b/scm/lily.scm
@@ -843,6 +843,7 @@ PIDs or the number of the process."
          (if (ly:get-option 'trace-memory-frequency)
              (mtrace:start-trace  (ly:get-option 'trace-memory-frequency)))
          (lilypond-file handler x)
+         (ly:check-expected-warnings)
          (if start-measurements
              (dump-profile x start-measurements (profile-measurements)))
          (if (ly:get-option 'trace-memory-frequency)