No more LUA_ERRGCMM errors

Errors in finalizers (__gc metamethods) are never propagated.
Instead, they generate a warning.

10 files changed, 145 insertions, 111 deletions

diff --git a/lapi.c b/lapi.c
index 0f0166e5..8ff7bfbd 100644
--- a/lapi.c
+++ b/lapi.c
@@ -1276,10 +1276,8 @@ void lua_setwarnf (lua_State *L, lua_WarnFunction f, void *ud) {
 
 
 void lua_warning (lua_State *L, const char *msg) {
-  lua_WarnFunction wf = G(L)->warnf;
   lua_lock(L);
-  if (wf != NULL)
-    wf(&G(L)->ud_warn, msg);
+  luaE_warning(L, msg);
   lua_unlock(L);
 }
 
diff --git a/lgc.c b/lgc.c
index 95a8ad5b..0c3386e7 100644
--- a/lgc.c
+++ b/lgc.c
@@ -824,7 +824,7 @@ static void dothecall (lua_State *L, void *ud) {
 }
 
 
-static void GCTM (lua_State *L, int propagateerrors) {
+static void GCTM (lua_State *L) {
   global_State *g = G(L);
   const TValue *tm;
   TValue v;
@@ -845,15 +845,13 @@ static void GCTM (lua_State *L, int propagateerrors) {
     L->ci->callstatus &= ~CIST_FIN;  /* not running a finalizer anymore */
     L->allowhook = oldah;  /* restore hooks */
     g->gcrunning = running;  /* restore state */
-    if (status != LUA_OK && propagateerrors) {  /* error while running __gc? */
-      if (status == LUA_ERRRUN) {  /* is there an error object? */
-        const char *msg = (ttisstring(s2v(L->top - 1)))
-                            ? svalue(s2v(L->top - 1))
-                            : "no message";
-        luaO_pushfstring(L, "error in __gc metamethod (%s)", msg);
-        status = LUA_ERRGCMM;  /* error in __gc metamethod */
-      }
-      luaD_throw(L, status);  /* re-throw error */
+    if (status != LUA_OK) {  /* error while running __gc? */
+      const char *msg = (ttisstring(s2v(L->top - 1)))
+                        ? svalue(s2v(L->top - 1))
+                        : "error object is not a string";
+      luaE_warning(L, "error in __gc metamethod (");
+      luaE_warning(L, msg);
+      luaE_warning(L, ")\n");
     }
   }
 }
@@ -866,7 +864,7 @@ static int runafewfinalizers (lua_State *L, int n) {
   global_State *g = G(L);
   int i;
   for (i = 0; i < n && g->tobefnz; i++)
-    GCTM(L, 1);  /* call one finalizer */
+    GCTM(L);  /* call one finalizer */
   return i;
 }
 
@@ -874,10 +872,10 @@ static int runafewfinalizers (lua_State *L, int n) {
 /*
 ** call all pending finalizers
 */
-static void callallpendingfinalizers (lua_State *L, int propagateerrors) {
+static void callallpendingfinalizers (lua_State *L) {
   global_State *g = G(L);
   while (g->tobefnz)
-    GCTM(L, propagateerrors);
+    GCTM(L);
 }
 
 
@@ -1124,7 +1122,7 @@ static void finishgencycle (lua_State *L, global_State *g) {
   checkSizes(L, g);
   g->gcstate = GCSpropagate;  /* skip restart */
   if (!g->gcemergency)
-    callallpendingfinalizers(L, 1);
+    callallpendingfinalizers(L);
 }
 
 
@@ -1334,7 +1332,7 @@ void luaC_freeallobjects (lua_State *L) {
   luaC_changemode(L, KGC_INC);
   separatetobefnz(g, 1);  /* separate all objects with finalizers */
   lua_assert(g->finobj == NULL);
-  callallpendingfinalizers(L, 0);
+  callallpendingfinalizers(L);
   deletelist(L, g->allgc, obj2gco(g->mainthread));
   deletelist(L, g->finobj, NULL);
   deletelist(L, g->fixedgc, NULL);  /* collect fixed objects */
diff --git a/lstate.c b/lstate.c
index b3e9ec60..7f6475a8 100644
--- a/lstate.c
+++ b/lstate.c
@@ -409,3 +409,10 @@ LUA_API void lua_close (lua_State *L) {
 }
 
 
+void luaE_warning (lua_State *L, const char *msg) {
+  lua_WarnFunction wf = G(L)->warnf;
+  if (wf != NULL)
+    wf(&G(L)->ud_warn, msg);
+}
+
+
diff --git a/lstate.h b/lstate.h
index f3793256..05a74dda 100644
--- a/lstate.h
+++ b/lstate.h
@@ -316,6 +316,7 @@ LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L);
 LUAI_FUNC void luaE_freeCI (lua_State *L);
 LUAI_FUNC void luaE_shrinkCI (lua_State *L);
 LUAI_FUNC void luaE_enterCcall (lua_State *L);
+LUAI_FUNC void luaE_warning (lua_State *L, const char *msg);
 
 
 #define luaE_exitCcall(L)       ((L)->nCcalls--)
diff --git a/ltests.c b/ltests.c
index 5ea8b080..0cb6d3a7 100644
--- a/ltests.c
+++ b/ltests.c
@@ -63,7 +63,11 @@ static void pushobject (lua_State *L, const TValue *o) {
 }
 
 
-static void badexit (void) {
+static void badexit (const char *fmt, ...) {
+  va_list argp;
+  va_start(argp, fmt);
+  vfprintf(stderr, fmt, argp);
+  va_end(argp);
   /* avoid assertion failures when exiting */
   l_memcontrol.numblocks = l_memcontrol.total = 0;
   exit(EXIT_FAILURE);
@@ -71,9 +75,9 @@ static void badexit (void) {
 
 
 static int tpanic (lua_State *L) {
-  fprintf(stderr, "PANIC: unprotected error in call to Lua API (%s)\n",
-                   lua_tostring(L, -1));
-  return (badexit(), 0);  /* do not return to Lua */
+  return (badexit("PANIC: unprotected error in call to Lua API (%s)\n",
+                   lua_tostring(L, -1)),
+          0);  /* do not return to Lua */
 }
 
 
@@ -83,16 +87,47 @@ static int islast (const char *message) {
 }
 
 
+/*
+** Warning function for tests. Fist, it concatenates all parts of
+** a warning in buffer 'buff'. Then:
+** messages starting with '#' are shown on standard output (used to
+** test explicit warnings);
+** messages containing '@' are stored in global '_WARN' (used to test
+** errors that generate warnings);
+** other messages abort the tests (they represent real warning conditions;
+** the standard tests should not generate these conditions unexpectedly).
+*/
 static void warnf (void **pud, const char *msg) {
-  if (*pud == NULL)  /* continuation line? */
-    printf("%s", msg);  /* print it */
-  else if (msg[0] == '*')  /* expected warning? */
-    printf("Expected Lua warning: %s", msg + 1);  /* print without the star */
-  else {  /* a real warning; should not happen during tests */
-    fprintf(stderr, "Warning in test mode (%s), aborting...\n", msg);
-    badexit();
-  }
-  *pud = islast(msg) ? pud : NULL;
+  static char buff[200];  /* should be enough for tests... */
+  static int cont = 0;  /* message to be continued */
+  if (cont) {  /* continuation? */
+    if (strlen(msg) >= sizeof(buff) - strlen(buff))
+      badexit("warnf-buffer overflow");
+    strcat(buff, msg);  /* add new message to current warning */
+  }
+  else {  /* new warning */
+    if (strlen(msg) >= sizeof(buff))
+      badexit("warnf-buffer overflow");
+    strcpy(buff, msg);  /* start a new warning */
+  }
+  if (!islast(msg))  /* message not finished yet? */
+    cont = 1;  /* wait for more */
+  else {  /* handle message */
+    cont = 0;  /* prepare for next message */
+    if (buff[0] == '#')  /* expected warning? */
+      printf("Expected Lua warning: %s", buff);  /* print it */
+    else if (strchr(buff, '@') != NULL) {  /* warning for test purposes? */
+      lua_State *L = cast(lua_State *, *pud);
+      lua_unlock(L);
+      lua_pushstring(L, buff);
+      lua_setglobal(L, "_WARN");  /* assign message to global '_WARN' */
+      lua_lock(L);
+      return;
+    }
+    else {  /* a real warning; should not happen during tests */
+      badexit("Unexpected warning in test mode: %s\naborting...\n", buff);
+    }
+  }
 }
 
 
diff --git a/lua.h b/lua.h
index a6f8268b..b777624e 100644
--- a/lua.h
+++ b/lua.h
@@ -51,8 +51,7 @@
 #define LUA_ERRRUN      2
 #define LUA_ERRSYNTAX   3
 #define LUA_ERRMEM      4
-#define LUA_ERRGCMM     5
-#define LUA_ERRERR      6
+#define LUA_ERRERR      5
 
 
 typedef struct lua_State lua_State;
diff --git a/manual/manual.of b/manual/manual.of
index 196ea1ef..d64f0f1a 100644
--- a/manual/manual.of
+++ b/manual/manual.of
@@ -722,8 +722,6 @@ Lua calls the finalizers of all objects marked for finalization,
 following the reverse order that they were marked.
 If any finalizer marks objects for collection during that phase,
 these marks have no effect.
-If any finalizer raises an error during that phase,
-its execution is interrupted but the error is ignored.
 
 Finalizers cannot yield.
 
@@ -2645,8 +2643,7 @@ by looking only at its arguments
 The third field, @T{x},
 tells whether the function may raise errors:
 @Char{-} means the function never raises any error;
-@Char{m} means the function may raise out-of-memory errors
-and errors running a finalizer;
+@Char{m} means the function may raise only out-of-memory errors;
 @Char{v} means the function may raise the errors explained in the text;
 @Char{e} means the function can run arbitrary Lua code,
 either directly or through metamethods,
@@ -3364,12 +3361,6 @@ syntax error during precompilation;}
 @item{@Lid{LUA_ERRMEM}|
 @x{memory allocation (out-of-memory) error};}
 
-@item{@Lid{LUA_ERRGCMM}|
-error while running a @idx{__gc} metamethod.
-(This error has no relation with the chunk being loaded.
-It is generated by the garbage collector.)
-}
-
 }
 
 The @id{lua_load} function uses a user-supplied @id{reader} function
@@ -3564,13 +3555,6 @@ For such errors, Lua does not call the @x{message handler}.
 error while running the @x{message handler}.
 }
 
-@item{@defid{LUA_ERRGCMM}|
-error while running a @idx{__gc} metamethod.
-For such errors, Lua does not call the @x{message handler}
-(as this kind of error typically has no relation
-with the function being called).
-}
-
 }
 
 }
@@ -6298,6 +6282,8 @@ The current value of this variable is @St{Lua 5.4}.
 @LibEntry{warn (message)|
 
 Emits a warning with the given message.
+Note that messages not ending with an end-of-line
+are assumed to be continued by the message in the next call.
 
 }
 
@@ -8773,6 +8759,12 @@ so there is no need to check whether they are using the same
 address space.)
 }
 
+@item{
+The constant @Lid{LUA_ERRGCMM} was removed.
+Errors in finalizers are never propagated;
+instead, they generate a warning.
+}
+
 }
 
 }
diff --git a/testes/all.lua b/testes/all.lua
index bde4195e..506afad2 100644
--- a/testes/all.lua
+++ b/testes/all.lua
@@ -190,12 +190,17 @@ assert(dofile('verybig.lua', true) == 10); collectgarbage()
 dofile('files.lua')
 
 if #msgs > 0 then
-  warn("*tests not performed:\n  ")
+  warn("#tests not performed:\n  ")
   for i=1,#msgs do
     warn(msgs[i]); warn("\n  ")
   end
+  warn("\n")
 end
 
+print("(there should be two warnings now)")
+warn("#This is "); warn("an expected"); warn(" warning\n")
+warn("#This is"); warn(" another one\n")
+
 -- no test module should define 'debug'
 assert(debug == nil)
 
@@ -219,10 +224,6 @@ local _G, showmem, print, format, clock, time, difftime, assert, open =
 local fname = T and "time-debug.txt" or "time.txt"
 local lasttime
 
-
-warn("*This is "); warn("an expected"); warn(" warning\n")
-warn("*This is"); warn(" another one\n")
-
 if not usertests then
   -- open file with time of last performed test
   local f = io.open(fname)
diff --git a/testes/api.lua b/testes/api.lua
index b4d63866..893a36cb 100644
--- a/testes/api.lua
+++ b/testes/api.lua
@@ -114,13 +114,12 @@ end
 
 -- testing warnings
 T.testC([[
-  warning "*This "
-  warning "warning "
-  warning "should be in a"
-  warning " single line
+  warning "#This shold be a"
+  warning " single "
+  warning "warning
 "
-  warning "*This should be "
-  warning "another warning
+  warning "#This should be "
+  warning "another one
 "
 ]])
 
@@ -896,24 +895,15 @@ do   -- testing errors during GC
     a[i] = T.newuserdata(i)   -- creates several udata
   end
   for i=1,20,2 do   -- mark half of them to raise errors during GC
-    debug.setmetatable(a[i], {__gc = function (x) error("error inside gc") end})
+    debug.setmetatable(a[i],
+      {__gc = function (x) error("@expected error in gc") end})
   end
   for i=2,20,2 do   -- mark the other half to count and to create more garbage
     debug.setmetatable(a[i], {__gc = function (x) load("A=A+1")() end})
   end
+  a = nil
   _G.A = 0
-  a = 0
-  while 1 do
-    local stat, msg = pcall(collectgarbage)
-    if stat then
-      break   -- stop when no more errors
-    else
-      a = a + 1
-      assert(string.find(msg, "__gc"))
-    end
-  end
-  assert(a == 10)  -- number of errors
-
+  collectgarbage()
   assert(A == 10)  -- number of normal collections
   collectgarbage("restart")
 end
diff --git a/testes/gc.lua b/testes/gc.lua
index 8b9179c8..84e8ffb7 100644
--- a/testes/gc.lua
+++ b/testes/gc.lua
@@ -353,40 +353,36 @@ GC()
 
 
 -- testing errors during GC
-do
-collectgarbage("stop")   -- stop collection
-local u = {}
-local s = {}; setmetatable(s, {__mode = 'k'})
-setmetatable(u, {__gc = function (o)
-  local i = s[o]
-  s[i] = true
-  assert(not s[i - 1])   -- check proper finalization order
-  if i == 8 then error("here") end   -- error during GC
-end})
-
-for i = 6, 10 do
-  local n = setmetatable({}, getmetatable(u))
-  s[n] = i
-end
-
-assert(not pcall(collectgarbage))
-for i = 8, 10 do assert(s[i]) end
-
-for i = 1, 5 do
-  local n = setmetatable({}, getmetatable(u))
-  s[n] = i
-end
+if T then
+  collectgarbage("stop")   -- stop collection
+  local u = {}
+  local s = {}; setmetatable(s, {__mode = 'k'})
+  setmetatable(u, {__gc = function (o)
+    local i = s[o]
+    s[i] = true
+    assert(not s[i - 1])   -- check proper finalization order
+    if i == 8 then error("@expected@") end   -- error during GC
+  end})
+
+  for i = 6, 10 do
+    local n = setmetatable({}, getmetatable(u))
+    s[n] = i
+  end
 
-collectgarbage()
-for i = 1, 10 do assert(s[i]) end
+  collectgarbage()
+  assert(string.find(_WARN, "error in __gc metamethod"))
+  assert(string.match(_WARN, "@(.-)@") == "expected")
+  for i = 8, 10 do assert(s[i]) end
 
-getmetatable(u).__gc = false
+  for i = 1, 5 do
+    local n = setmetatable({}, getmetatable(u))
+    s[n] = i
+  end
 
+  collectgarbage()
+  for i = 1, 10 do assert(s[i]) end
 
--- __gc errors with non-string messages
-setmetatable({}, {__gc = function () error{} end})
-local a, b = pcall(collectgarbage)
-assert(not a and type(b) == "string" and string.find(b, "error in __gc"))
+  getmetatable(u).__gc = false
 
 end
 print '+'
@@ -478,9 +474,11 @@ end
 
 
 -- errors during collection
-u = setmetatable({}, {__gc = function () error "!!!" end})
-u = nil
-assert(not pcall(collectgarbage))
+if T then
+  u = setmetatable({}, {__gc = function () error "@expected error" end})
+  u = nil
+  collectgarbage()
+end
 
 
 if not _soft then
@@ -645,11 +643,26 @@ do
 end
 
 -- create several objects to raise errors when collected while closing state
-do
-  local mt = {__gc = function (o) return o + 1 end}
-  for i = 1,10 do
+if T then
+  local error, assert, warn, find = error, assert, warn, string.find
+  local n = 0
+  local lastmsg
+  local mt = {__gc = function (o)
+    n = n + 1
+    assert(n == o[1])
+    if n == 1 then
+      _WARN = nil
+    elseif n == 2 then
+      assert(find(_WARN, "@expected warning"))
+      lastmsg = _WARN    -- get message from previous error (first 'o')
+    else
+      assert(lastmsg == _WARN)  -- subsequent error messages are equal
+    end
+    error"@expected warning"
+  end}
+  for i = 10, 1, -1 do
     -- create object and preserve it until the end
-    table.insert(___Glob, setmetatable({}, mt))
+    table.insert(___Glob, setmetatable({i}, mt))
   end
 end