Upload newest offline version (1.0.2)

Author: milochristiansen

README.md

@@ -6,9 +6,9 @@ This is a Lua 5.3 VM and compiler written in [Go](http://golang.org/). This is i
 programs, with minimal fuss and bother.
 
 I have been using this VM/compiler as the primary script host in Rubble (a scripted templating system used to generate
-data files for the game Dwarf Fortress) for over a year now, so they are fairly well tested. Speaking of tests: No formal
-unit tests are provided. Testing was done by running under real-world conditions with Rubble, and in Rubble's built-in
-template testing system (it is not appropriate to distribute these tests with this library for obvious reasons).
+data files for the game Dwarf Fortress) for over a year now, so they are fairly well tested. In addition to the real-world
+"testing" that this has received I am slowly adding proper tests based on the official Lua test suite. These tests are
+far from complete, but are slowly getting more so as time passes.
 
 Most (if not all) of the API functions may cause a panic, but only if things go REALLY wrong. If a function does not
 state that it can panic or "raise an error" it will only do so if a critical internal assumption proves to be wrong
@@ -47,16 +47,45 @@ If you want to use a third-party compiler it will need to produce binaries with
 
 When building the reference compiler on most systems these settings should be the default.
 
+The VM API has a function that wraps `luac` to load code, but the way it does this may or may not fit your needs. To use
+this wrapper you will need to have `luac` on your path or otherwise placed so the VM can find it. See the documentation
+for `State.LoadTextExternal` for more information. Keep in mind that due to limitations in Go and `luac`, this function
+is not reentrant! If you need concurrency support it would be better to use `State.LoadBinary` and write your own wrapper.
+
 The default compiler provided by this library does not support constant folding, and some special instructions are not
 used at all (instead preferring simpler sequences of other instructions). For example TESTSET is never generated, TEST
 is used in all cases (largely because it would greatly complicate the compiler if I tried to use TESTSET where possible).
 Expressions use a simple "recursive" code generation style, meaning that it wastes registers like crazy in some (rare)
 cases.
 
+One of the biggest code quality offenders is `or` and `and`, as they can result in sequences like this one:
+
+	[4]   LT        A:1  B:r(0)   C:k(2)  ; CK:5
+	[5]   JMP       A:0  SBX:1            ; to:7
+	[6]   LOADBOOL  A:2  B:1      C:1
+	[7]   LOADBOOL  A:2  B:0      C:0
+	[8]   TEST      A:2           C:1
+	[9]   JMP       A:0  SBX:7            ; to:17
+	[10]  EQ        A:1  B:r(1)   C:k(3)  ; CK:<nil>
+	... (7 more instructions to implement next part of condition)
+
+As you can see this is terrible. That sequence would be better written as:
+
+	[4]   LT        A:1  B:r(0)   C:k(2)  ; CK:5
+	[5]   JMP       A:0  SBX:2            ; to:8
+	[6]   EQ        A:1  B:r(1)   C:k(3)  ; CK:<nil>
+	... (1 more instruction to implement next part of condition)
+
+But the current expression compiler is not smart enough to do it that way. Luckily this is the worst offender, most
+things produce code that is very close or identical to what `luac` produces. Note that the reason why this code is so
+bad is entirely because the expression used `or` (and the implementation of `and` and `or` is very bad).
+
 The compiler provides an implementation of a `continue` keyword, but the keyword definition in the lexer is commented
 out. If you want `continue` all you need to do is uncomment the indicated line (near the top of `ast/lexer.go`). There
 is also a flag in the VM that *should* make tables use 0 based indexing. This feature has received minimal testing, so
 it probably doesn't work properly. If you want to try 0 based indexing just set the variable `TableIndexOffset` to 0.
+Note that `TableIndexOffset` is strictly a VM setting, the standard modules do not respect this setting (for example the
+`table` module and `ipairs` will still insist on using 1 as the first index).
 
 
 Missing Stuff:
@@ -142,6 +171,34 @@ to rework the error handler...
 Changes:
 ------------------------------------------------------------------------------------------------------------------------
 
+1.0.2
+
+More tests, more (compiler) bugs fixed. Damn compiler will be the death of me yet...
+
+In addition to the inevitable compiler bugs I also fixed the way the VM handles upvalues. Before I was giving each
+closure its own copy of each upvalue, so multiple closures never properly shared values. This change fixes several
+subtle (and several not so subtle) bugs.
+
+Oh, and `pcall` works now (it didn't work at all before. Sorry, I never used it).
+
+* Added more script tests. I still have a lot more to do... (script_test.go)
+* Fixed incorrect compilation of method declarations (`function a:x() end`). Depressingly the issue was only one
+  incorrect word, but it resulted in *very* wrong results (I am really starting to remember why I hated writing the
+  compiler, the VM was fun, the compiler... not.) (ast/parse.go)
+* Parenthesized expression that would normally (without the parenthesis) return multiple values (for example: `(...)`)
+  were not properly truncating the result to a single value. (compile_expr.go)
+* Fixed a semi-major VM issue with upvalues. Closures that should have a single shared upvalue were instead each using
+  their own private copy after said upvalue was closed. This required an almost total rewrite of the way upvalues are
+  stored internally. (all over the place, but mainly callframe.go, function.go, api.go, and vm.go)
+* JMP instructions created by `break` and `continue` statements are now properly patched by the compiler to close any
+  upvalues there may be. (compile.go)
+* Fixed the `pcall` script function so it actually works. (lmodbase/functions.go)
+* On a recovered error each stack frame's upvalues are closed before the stack is stripped. This corrects incorrect
+  behavior that arises when a function stores a closure to an unclosed upvalue then errors out (the closure may still be
+  referenced, but it's upvalues may be invalid). (api.go, callframe.go)
+
+* * *
+
 1.0.1
 
 This version adds a bunch of tests (still not nearly as many as I would like), and fixes a ton of minor compiler errors.

api.go

@@ -55,18 +55,26 @@ func (l *State) Push(v interface{}) {
 	case func(l *State) int:
 		v = &function{
 			native: v2,
-			uvDefs: []upValue{{name: "_ENV", index: -1}},
-			uvClosed: []bool{true},
-			upVals: []value{l.global},
-			uvAbsIdxs: []int{-1},
+			up: []*upValue{{
+					name: "_ENV",
+					index: -1,
+					closed: true,
+					val: l.global,
+					absIdx: -1,
+				},
+			},
 		}
 	case NativeFunction:
 		v = &function{
 			native: v2,
-			uvDefs: []upValue{{name: "_ENV", index: -1}},
-			uvClosed: []bool{true},
-			upVals: []value{l.global},
-			uvAbsIdxs: []int{-1},
+			up: []*upValue{{
+					name: "_ENV",
+					index: -1,
+					closed: true,
+					val: l.global,
+					absIdx: -1,
+				},
+			},
 		}
 	default:
 		v = &userData{
@@ -88,27 +96,27 @@ func (l *State) PushClosure(f NativeFunction, v ...int) {
 
 	fn := &function{
 		native: f,
-		uvDefs: make([]upValue, c),
-		uvClosed: make([]bool, c),
-		upVals: make([]value, c),
-		uvAbsIdxs: make([]int, c),
+		up: make([]*upValue, c),
 	}
 
 	// ALL native functions ALWAYS have their first upvalue set to the global table.
 	// This differs from standard Lua, but doesn't hurt anything.
-	fn.uvDefs[0].name = "_ENV"
-	fn.uvDefs[0].index = 0
-	fn.uvClosed[0] = true
-	fn.upVals[0] = l.global
-	fn.uvAbsIdxs[0] = -1
+	fn.up[0] = &upValue{
+		name: "_ENV",
+		index: -1,
+		closed: true,
+		val: l.global,
+		absIdx: -1,
+	}
 
 	for i := 1; i < c; i++ {
-		fn.uvDefs[i].name = "(native upvalue)"
-		fn.uvDefs[i].index = -1
-		fn.uvClosed[i] = true
-		val := l.get(v[i-1])
-		fn.upVals[i] = val
-		fn.uvAbsIdxs[i] = -1
+		fn.up[i] = &upValue{
+			name: "(native upvalue)",
+			index: -1,
+			closed: true,
+			val: l.get(v[i-1]),
+			absIdx: -1,
+		}
 	}
 
 	l.stack.Push(fn)
@@ -518,16 +526,22 @@ func (l *State) ForEachInTable(t int, f func()) {
 // Other
 
 // SetUpVal sets upvalue "i" in the function at "f" to the value at "v".
-// If the upvalue index is out of range or "f" is not a function false is returned
-// and nothing is done, else returns true and sets the upval.
+// If the upvalue index is out of range, "f" is not a function, or the upvalue
+// is not closed false is returned and nothing is done, else returns true and
+// sets the upvalue.
+// 
+// Any other functions that share this upvalue will also be affected!
 func (l *State) SetUpVal(f, i, v int) bool {
 	fn, ok := l.get(f).(*function)
-	if !ok || i >= len(fn.uvDefs) {
+	if !ok || i >= len(fn.up) {
 		return false
 	}
 
-	fn.uvClosed[i] = true
-	fn.upVals[i] = l.get(v)
+	def := fn.up[i]
+	if !def.closed {
+		return false
+	}
+	def.val = l.get(v)
 	return true
 }
 
@@ -772,20 +786,29 @@ func (l *State) ListFunc(i int) {
 
 // Execution
 
+// Used to create the return values for the compiler API functions (nothing else!).
 func (l *State) asFunc(proto *funcProto, env *table) *function {
 	f := &function{
 		proto: *proto,
-		uvDefs: proto.upVals,
-		uvClosed: make([]bool, len(proto.upVals)),
-		upVals: make([]value, len(proto.upVals)),
-		uvAbsIdxs: make([]int, len(proto.upVals)),
+		up: make([]*upValue, len(proto.upVals)),
 	}
-	for i := range f.uvAbsIdxs {
-		f.uvAbsIdxs[i] = -1
+	for i := range f.up {
+		def := proto.upVals[i].makeUp()
+		
+		// Don't set name or index! name may come in from debug info, index is meaningless when closed.
+		def.closed = true
+		def.absIdx = -1
+		f.up[i] = def
 	}
 
-	f.uvClosed[0] = true
-	f.upVals[0] = env
+	// Top level functions must have their first upvalue as _ENV
+	if len(f.up) > 0 {
+		if f.up[0].name != "_ENV" && f.up[0].name != "" {
+			luautil.Raise("Top level function without _ENV or _ENV in improper position.", luautil.ErrTypGenRuntime)
+		}
+		
+		f.up[0].val = env
+	}
 
 	return f
 }
@@ -848,7 +871,7 @@ func (l *State) LoadText(in io.Reader, name string, env int) error {
 // This version looks for and runs "luac" to compile the chunk. Make sure luac is on
 // your path.
 // 
-// This functions is not safe for concurrent use.
+// This function is not safe for concurrent use.
 func (l *State) LoadTextExternal(in io.Reader, name string, env int) error {
 	outFile := os.TempDir() + "/dctech.lua.bin" // Go seems to lack a function to get a temporary file name, so this is unsafe for concurrent use!
 	cmd := exec.Command("luac", "-o", outFile, "-")
@@ -946,6 +969,10 @@ func (l *State) PCall(args, rtns int) (err error) {
 				trace = fmt.Sprintf("%v\n\nNative Trace:\n%s\n", trace, buf)
 			}
 
+			// Before we strip the stack we need to close all upvalues in the section we will be stripping, just in
+			// case a closure was assigned to another upvalue.
+			l.stack.frames[len(l.stack.frames) - 1].closeUpAbs(top)
+			
 			// Make sure the stack is back to the way we found it, minus the function and it's arguments.
 			l.stack.frames = l.stack.frames[:frames]
 			for i := len(l.stack.data) - 1; i >= top; i-- {

ast/parse.go

@@ -88,7 +88,7 @@ func (p *parser) funcDeclStat(local bool) Stmt {
 			p.l.getCurrent(tknName)
 			ident = exprLine(&TableAccessor{
 				Obj: ident,
-				Key: exprLine(&ConstIdent{
+				Key: exprLine(&ConstString{
 					Value: p.l.current.Lexeme,
 				}, p.l.current.Line),
 			}, line)