Improve accidentally quadratic LValue foldability checks

libtailslide/builtins.cc

@@ -13,7 +13,7 @@ extern const char *BUILTINS_TXT[];
 static ScriptAllocator gStaticAllocator {};
 
 // holds the symbols for the default builtins
-LSLSymbolTable gBuiltinsSymbolTable{nullptr}; // NOLINT(cert-err58-cpp)
+LSLSymbolTable gBuiltinsSymbolTable{nullptr, SYMTAB_BUILTINS}; // NOLINT(cert-err58-cpp)
 
 struct LSLTypeMap {
     const char *name;

libtailslide/loctype.hh

@@ -8,6 +8,32 @@ struct TailslideLType {
   int first_column;
   int last_line;
   int last_column;
+
+  bool operator>(const TailslideLType &other) const {
+    if (first_line > other.first_line)
+      return true;
+    if (first_line < other.first_line)
+      return false;
+
+    if (first_column > other.first_column)
+      return true;
+    if (first_column < other.first_column)
+      return false;
+    return false;
+  }
+
+  bool operator<(const TailslideLType &other) const {
+    if (first_line < other.first_line)
+      return true;
+    if (first_line > other.first_line)
+      return false;
+
+    if (first_column < other.first_column)
+      return true;
+    if (first_column > other.first_column)
+      return false;
+    return false;
+  }
 };
 
 // keep existing consumers inside tailslide working

libtailslide/passes/symbol_resolution.cc

@@ -4,7 +4,7 @@ namespace Tailslide {
 
 
 bool SymbolResolutionVisitor::visit(LSLScript *script) {
-  replaceSymbolTable(script);
+  replaceSymbolTable(script, SYMTAB_GLOBAL);
   auto *globals = script->getGlobals();
   // all global var definitions are implicitly hoisted above function definitions
   // all functions and states have their declarations implicitly hoisted as well.
@@ -15,7 +15,7 @@ bool SymbolResolutionVisitor::visit(LSLScript *script) {
     } else if (global->getNodeType() == NODE_GLOBAL_FUNCTION) {
       // just record the prototype of the function and don't descend for now.
       auto *global_func = (LSLGlobalFunction *)global;
-      replaceSymbolTable(global_func);
+      replaceSymbolTable(global_func, SYMTAB_FUNCTION);
       auto *identifier = global_func->getIdentifier();
 
       // define function in script scope since functions have their own scope
@@ -34,7 +34,7 @@ bool SymbolResolutionVisitor::visit(LSLScript *script) {
   // now walk the states to register their prototypes
   auto *states = script->getStates();
   for (auto *state : *states) {
-    replaceSymbolTable(state);
+    replaceSymbolTable(state, SYMTAB_STATE);
     auto *identifier = ((LSLState *)state)->getIdentifier();
     identifier->setSymbol(_mAllocator->newTracked<LSLSymbol>(
         identifier->getName(), identifier->getType(), SYM_STATE, SYM_GLOBAL, identifier->getLoc()
@@ -87,9 +87,9 @@ bool SymbolResolutionVisitor::visit(LSLDeclaration *decl_stmt) {
 }
 
 /// replace the node's old symbol table, registering the new one.
-void SymbolResolutionVisitor::replaceSymbolTable(LSLASTNode *node) {
+void SymbolResolutionVisitor::replaceSymbolTable(LSLASTNode *node, LSLSymbolTableType symtab_type) {
   // TODO: unregister old table? need to figure out node copy semantics.
-  auto *symtab = _mAllocator->newTracked<LSLSymbolTable>();
+  auto *symtab = _mAllocator->newTracked<LSLSymbolTable>(symtab_type);
   node->setSymbolTable(symtab);
   node->mContext->table_manager->registerTable(symtab);
 }
@@ -107,12 +107,13 @@ bool SymbolResolutionVisitor::visit(LSLFunctionExpression *func_expr) {
 bool SymbolResolutionVisitor::visit(LSLGlobalFunction *glob_func) {
   assert(_mPendingJumps.empty());
   visitChildren(glob_func);
+  glob_func->getSymbolTable()->setLabels(_mCollectedLabels);
   resolvePendingJumps(glob_func);
   return false;
 }
 
 bool SymbolResolutionVisitor::visit(LSLEventHandler *handler) {
-  replaceSymbolTable(handler);
+  replaceSymbolTable(handler, SYMTAB_FUNCTION);
 
   auto *id = handler->getIdentifier();
   // look for a prototype for this event in the builtin namespace
@@ -128,6 +129,7 @@ bool SymbolResolutionVisitor::visit(LSLEventHandler *handler) {
 
   assert(_mPendingJumps.empty());
   visitChildren(handler);
+  handler->getSymbolTable()->setLabels(_mCollectedLabels);
   resolvePendingJumps(handler);
   return false;
 }
@@ -183,7 +185,7 @@ bool SymbolResolutionVisitor::visit(LSLStateStatement *state_stmt) {
 }
 
 bool SymbolResolutionVisitor::visit(LSLCompoundStatement *compound_stmt) {
-  replaceSymbolTable(compound_stmt);
+  replaceSymbolTable(compound_stmt, SYMTAB_LEXICAL);
   return true;
 }
 

libtailslide/passes/symbol_resolution.hh

@@ -32,7 +32,7 @@ class SymbolResolutionVisitor : public ASTVisitor {
     virtual bool visit(LSLForStatement *for_stmt);
     void visitLoop(LSLASTNode *loop_stmt);
 
-    void replaceSymbolTable(LSLASTNode *node);
+    void replaceSymbolTable(LSLASTNode *node, LSLSymbolTableType symtab_type);
 
     void resolvePendingJumps(LSLASTNode *func_like);
     ScriptAllocator *_mAllocator;

libtailslide/passes/type_checking.cc

@@ -395,30 +395,43 @@ bool TypeCheckVisitor::visit(LSLLValueExpression *lvalue) {
   // between us and its declaration that'd make it unfoldable
   if (symbol->getSubType() == SYM_LOCAL && symbol->getVarDecl() != nullptr) {
     LSLASTNode *local_decl = symbol->getVarDecl();
+    assert(local_decl != nullptr);
 
-    // walk up and find the statement at the top of this expression;
+    // Walk up and find the symbol table for the function this is in
     LSLASTNode *upper_node = lvalue->getParent();
-    while (upper_node != nullptr && upper_node->getNodeType() != NODE_STATEMENT) {
+    LSLSymbolTable *func_symtab = nullptr;
+    while (upper_node != nullptr) {
+      auto *symtab = upper_node->getSymbolTable();
+      if (symtab && symtab->getTableType() == SYMTAB_FUNCTION) {
+        func_symtab = symtab;
+        break;
+      }
       upper_node = upper_node->getParent();
     }
-    if (upper_node != nullptr) {
-      auto *parent_stmt = (LSLStatement *) upper_node;
-      auto *found_stmt = (LSLStatement *) parent_stmt->findPreviousInScope(
-          [local_decl](LSLASTNode *to_check) {
-            // stop searching once we hit the declaration or a label
-            return to_check == local_decl || to_check->getNodeSubType() == NODE_LABEL;
-          }
-      );
+
+    // LValue ref for a local should always be within a function
+    assert(func_symtab != nullptr);
+
+    bool is_foldable = true;
+    if (*local_decl->getLoc() > *lvalue->getLoc()) {
       // the declaration really should be above us somewhere
       // unless they did something screwy like `if(r)string r = baz;`
-
-      // hit a label before the declaration? Not gonna inline it.
-      // we could be smarter about this, but it's probably not worth it.
-      // we could check if there was also a jump to that label somewhere
-      // before us in the same scope (before the declaration obviously.)
-      lvalue->setIsFoldable(found_stmt && (found_stmt->getNodeSubType() != NODE_LABEL));
-      return false;
+      is_foldable = false;
+    } else {
+      // check if a label occurs between the declaration and lvalue
+      for (auto *label_ptr : func_symtab->getLabels()) {
+        // hit a label before the declaration? Not gonna inline it.
+        // we could be smarter about this, but it's probably not worth it.
+        // we could check if there was also a jump to that label somewhere
+        // before us in the same scope (before the declaration obviously.)
+        if (*label_ptr->getLoc() > *local_decl->getLoc() && *label_ptr->getLoc() < *lvalue->getLoc()) {
+          is_foldable = false;
+          break;
+        }
+      }
     }
+    lvalue->setIsFoldable(is_foldable);
+    return false;
   }
   lvalue->setIsFoldable(true);
   return false;

libtailslide/symtab.hh

@@ -1,13 +1,14 @@
 #ifndef TAILSLIDE_SYMTAB_HH
 #define TAILSLIDE_SYMTAB_HH
 
+#include <cassert>
 #include <clocale>
 #include <cstddef>
-#include <vector>
 #include <unordered_map>
-#include "unordered_cstr_map.hh"
+#include <vector>
 
 #include "allocator.hh"
+#include "unordered_cstr_map.hh"
 
 namespace Tailslide {
 
@@ -25,6 +26,7 @@ enum LSLIType : uint8_t {
 };
 
 enum LSLSymbolType       { SYM_ANY = -1, SYM_VARIABLE, SYM_FUNCTION, SYM_STATE, SYM_LABEL, SYM_EVENT };
+enum LSLSymbolTableType  { SYMTAB_GLOBAL, SYMTAB_STATE, SYMTAB_FUNCTION, SYMTAB_LEXICAL, SYMTAB_BUILTINS };
 enum LSLSymbolSubType    { SYM_LOCAL, SYM_GLOBAL, SYM_BUILTIN, SYM_FUNCTION_PARAMETER, SYM_EVENT_PARAMETER };
 
 class LSLSymbol: public TrackableObject {
@@ -103,18 +105,33 @@ class LSLSymbol: public TrackableObject {
 
 class LSLSymbolTable: public TrackableObject {
   public:
-    explicit LSLSymbolTable(ScriptContext *ctx): TrackableObject(ctx) {};
+    explicit LSLSymbolTable(ScriptContext *ctx, LSLSymbolTableType symtab_type)
+      : TrackableObject(ctx), _mSymbolTableType(symtab_type) {};
     LSLSymbol *lookup( const char *name, LSLSymbolType type = SYM_ANY );
     void            define( LSLSymbol *symbol );
     bool            remove( LSLSymbol *symbol );
     void            checkSymbols();
     void resetTracking();
 
   private:
-    UnorderedCStrMap<LSLSymbol*> _mSymbols;
+    UnorderedCStrMap<LSLSymbol *> _mSymbols;
+    std::vector<class LSLLabel *> _mLabels;
+    LSLSymbolTableType _mSymbolTableType;
 
   public:
     UnorderedCStrMap<LSLSymbol*> &getMap() {return _mSymbols;}
+    LSLSymbolTableType getTableType() { return _mSymbolTableType; }
+
+    // Used for tracking all labels in a function. Labels in LSL are
+    // lexically scoped by specification, function scoped by implementation :(
+    void setLabels(const std::vector<class LSLLabel *> &labels) {
+      assert(_mSymbolTableType == SYMTAB_FUNCTION);
+      _mLabels = labels;
+    }
+    const std::vector<class LSLLabel *> &getLabels() {
+      assert(_mSymbolTableType == SYMTAB_FUNCTION);
+      return _mLabels;
+    }
 };
 
 class LSLSymbolTableManager {

tests/unit_tests.cc

@@ -122,4 +122,14 @@ TEST_CASE("Bitstream read_only")
   CHECK(bs2.isReadOnly());
 }
 
+TEST_CASE("LLoc comparison works correctly") {
+  TailslideLType smaller {0, 1, 2, 3};
+  TailslideLType bigger {1, 1, 2, 3};
+  TailslideLType smaller_sameline {1, 0, 2, 3};
+  CHECK(bigger > smaller);
+  CHECK(bigger > smaller_sameline);
+  CHECK(smaller < bigger);
+  CHECK(smaller_sameline < bigger);
+}
+
 TEST_SUITE_END();