neolithos / neolua

A Lua implementation for the Dynamic Language Runtime (DLR).
https://neolua.codeplex.com/
Apache License 2.0
466 stars 76 forks source link

Empty if block and DebugEngine = LuaExceptionDebugger.Default causes exception in LuaExceptionVisitor #155

Closed thoj closed 2 years ago

thoj commented 2 years ago

NeoLua Version: master

I am bit out on deep water here. I was trying to debug a Callback function. And the StacktraceDebugger did not emit a stack trace so i switched DebugEngine to LuaExceptionDebugger. I get the exact same error when using LuaTraceLineDebugger. Running my script i got this long error:

System.ArgumentException: Non-empty collection required
Parameter name: expressions
   at System.Dynamic.Utils.ContractUtils.RequiresNotEmpty[T](ICollection`1 collection, String paramName)
   at System.Linq.Expressions.Expression.Block(Type type, IEnumerable`1 variables, IEnumerable`1 expressions)
   at Neo.IronLua.LuaExceptionDebugger.LuaExceptionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.BlockExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitConditional(ConditionalExpression node)
   at System.Linq.Expressions.ConditionalExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at Neo.IronLua.LuaExceptionDebugger.LuaExceptionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.BlockExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitTry(TryExpression node)
   at System.Linq.Expressions.TryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at Neo.IronLua.LuaExceptionDebugger.LuaExceptionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.BlockExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitLambda[T](Expression`1 node)
   at Neo.IronLua.LuaExceptionDebugger.LuaExceptionVisitor.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.Expression`1.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Neo.IronLua.LuaExceptionDebugger.LuaExceptionChunk..ctor(Lua lua, LambdaExpression expr)
   at Neo.IronLua.LuaExceptionDebugger.Neo.IronLua.ILuaDebug.CreateChunk(Lua lua, LambdaExpression expr)
   at Neo.IronLua.Lua.CompileChunkCore(ILuaLexer lex, LuaCompileOptions options, IEnumerable`1 args)
   at Neo.IronLua.Lua.CompileChunk(String fileName, LuaCompileOptions options, KeyValuePair`2[] args)
   at NectOPC.NectOPC.ExecuteLua(String scriptfile)

After putting some print statements on LuaExceptionVisitor i got the line number where this error happened and i made this minimal example:

Example to reproduce:

lua.CompileChunk(scriptfile, new LuaCompileOptions() { DebugEngine = LuaExceptionDebugger.Default });
local foo = true

if foo then
  print("True")
else
--nop
end

If i use LuaStackTraceDebugger.Default. This example runs fine. I'm pretty sure an empty block like this is valid Lua. So i think this is a bug.

If remove the empty block the script runs fine with LuaExceptionDebugger and i actually got the correct line number with the error in my callback :)

thoj commented 2 years ago
diff --git a/NeoLua/LuaDebug.cs b/NeoLua/LuaDebug.cs
index 9c0ed6a..9249f74 100644
--- a/NeoLua/LuaDebug.cs
+++ b/NeoLua/LuaDebug.cs
@@ -196,7 +196,12 @@ namespace Neo.IronLua
                        } // func LineProgressExpression

                        protected override Expression VisitBlock(BlockExpression node)
-                               => base.VisitBlock(Expression.Block(node.Type, node.Variables, LineProgressExpression(node.Expressions)));
+                       {
+                               var expr = LineProgressExpression(node.Expressions);
+                               if (expr.Count() == 0) return Expression.Empty();
+
+                               return base.VisitBlock(Expression.Block(node.Type, node.Variables, expr));
+                       }

                        protected override Expression VisitDebugInfo(DebugInfoExpression node)
                                => throw new InvalidOperationException();

This fixes it as far as I can tell. All tests passes but there may be some side effects that I'm not aware of.

neolithos commented 2 years ago

Thank you for the fix. I need to clean the master, before I can put this in.