From a6739819b2f54d02a5d315635a50974ac1c54f34 Mon Sep 17 00:00:00 2001
From: Bob Nystrom <robert@stuffwithstuff.com>
Date: Wed, 16 Mar 2016 08:00:28 -0700
Subject: [PATCH] Report undefined variable errors on the line where they are
 used.

This also more importantly ensures the error message for one has a
bounded length even if the variable name is pathologically long.
---
 src/vm/wren_compiler.c                | 17 +++++++++++------
 src/vm/wren_vm.c                      | 12 ++++++++----
 src/vm/wren_vm.h                      |  5 +++--
 test/language/nonlocal/undefined.wren |  6 ++----
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/vm/wren_compiler.c b/src/vm/wren_compiler.c
index 231e0180..9e5efb16 100644
--- a/src/vm/wren_compiler.c
+++ b/src/vm/wren_compiler.c
@@ -2213,7 +2213,8 @@ static void name(Compiler* compiler, bool canAssign)
     // the hopes that we get a real definition later.
     variable.index = wrenDeclareVariable(compiler->parser->vm,
                                          compiler->parser->module,
-                                         token->start, token->length);
+                                         token->start, token->length,
+                                         token->line);
 
     if (variable.index == -2)
     {
@@ -3438,14 +3439,18 @@ ObjFn* wrenCompile(WrenVM* vm, ObjModule* module, const char* source,
   emitOp(&compiler, CODE_RETURN);
 
   // See if there are any implicitly declared module-level variables that never
-  // got an explicit definition.
-  // TODO: It would be nice if the error was on the line where it was used.
+  // got an explicit definition. They will have values that are numbers
+  // indicating the line where the variable was first used.
   for (int i = 0; i < parser.module->variables.count; i++)
   {
-    if (IS_UNDEFINED(parser.module->variables.data[i]))
+    if (IS_NUM(parser.module->variables.data[i]))
     {
-      error(&compiler, "Variable '%s' is used but not defined.",
-            parser.module->variableNames.data[i].buffer);
+      // Synthesize a token for the original use site.
+      parser.previous.type = TOKEN_NAME;
+      parser.previous.start = parser.module->variableNames.data[i].buffer;
+      parser.previous.length = parser.module->variableNames.data[i].length;
+      parser.previous.line = (int)AS_NUM(parser.module->variables.data[i]);
+      error(&compiler, "Variable is used but not defined.");
     }
   }
   
diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c
index 2be9c497..8178901a 100644
--- a/src/vm/wren_vm.c
+++ b/src/vm/wren_vm.c
@@ -1363,11 +1363,14 @@ Value wrenFindVariable(WrenVM* vm, ObjModule* module, const char* name)
 }
 
 int wrenDeclareVariable(WrenVM* vm, ObjModule* module, const char* name,
-                        size_t length)
+                        size_t length, int line)
 {
   if (module->variables.count == MAX_MODULE_VARS) return -2;
 
-  wrenValueBufferWrite(vm, &module->variables, UNDEFINED_VAL);
+  // Implicitly defined variables get a "value" that is the line where the
+  // variable is first used. We'll use that later to report an error on the
+  // right line.
+  wrenValueBufferWrite(vm, &module->variables, NUM_VAL(line));
   return wrenSymbolTableAdd(vm, &module->variableNames, name, length);
 }
 
@@ -1387,9 +1390,10 @@ int wrenDefineVariable(WrenVM* vm, ObjModule* module, const char* name,
     symbol = wrenSymbolTableAdd(vm, &module->variableNames, name, length);
     wrenValueBufferWrite(vm, &module->variables, value);
   }
-  else if (IS_UNDEFINED(module->variables.data[symbol]))
+  else if (IS_NUM(module->variables.data[symbol]))
   {
-    // Explicitly declaring an implicitly declared one. Mark it as defined.
+    // An implicitly declared variable's value will always be a number. Now we
+    // have a real definition.
     module->variables.data[symbol] = value;
   }
   else
diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h
index ccf3c779..a133893a 100644
--- a/src/vm/wren_vm.h
+++ b/src/vm/wren_vm.h
@@ -154,13 +154,14 @@ Value wrenGetModuleVariable(WrenVM* vm, Value moduleName, Value variableName);
 // module.
 Value wrenFindVariable(WrenVM* vm, ObjModule* module, const char* name);
 
-// Adds a new implicitly declared top-level variable named [name] to [module].
+// Adds a new implicitly declared top-level variable named [name] to [module]
+// based on a use site occurring on [line].
 //
 // Does not check to see if a variable with that name is already declared or
 // defined. Returns the symbol for the new variable or -2 if there are too many
 // variables defined.
 int wrenDeclareVariable(WrenVM* vm, ObjModule* module, const char* name,
-                        size_t length);
+                        size_t length, int line);
 
 // Adds a new top-level variable named [name] to [module].
 //
diff --git a/test/language/nonlocal/undefined.wren b/test/language/nonlocal/undefined.wren
index 2ff78950..cf5527c8 100644
--- a/test/language/nonlocal/undefined.wren
+++ b/test/language/nonlocal/undefined.wren
@@ -1,6 +1,4 @@
 var fn = Fn.new {
-  System.print(Foo)
-  System.print(Bar)
+  System.print(Foo) // expect error
+  System.print(Bar) // expect error
 }
-
-// expect error line 6
\ No newline at end of file