fix(core+std): fix panic with index out of range for string slice (#47)

Improve `string` code and avoid panic due to integer overflow.

The `std.fs.Path.dirname` and `std.fs.Path.basename` functions are also renamed to `std.fs.Path.dir_name` and `std.fs.Path.base_name` respectively.

A new function is also added: `std.fs.Path.file_name`, to get the file name from some path.

Clang sporadically fails tests for `std.fs.Path.dir_name()`. I still don't know the reason, but it's strange. GCC runs everything well.

cmd/src/tools/cmd_new.ri

@@ -61,7 +61,7 @@ public func new(args: []string, is_init: bool) -> ! {
 
     mut project := build.Project();
     if is_init {
-        project.name = Path.basename(process.get_cwd()!);
+        project.name = Path.base_name(process.get_cwd()!);
     } else {
         if remaining.len == 1 {
             project.name = remaining[0];

lib/core/src/string.c.ri

@@ -210,10 +210,40 @@ public struct string < Stringable, Hashable, Throwable {
     /// Returns the contents before `sub` in the string.
     /// If the substring is not found, it returns the full input string.
     public func all_before_of(self, sub: Self) -> Self {
-        if pos := self.index_of(sub) {
-            return self[..pos];
-        }
-        return self;
+        return if pos := self.index_of(sub) {
+            self[..pos]
+        } else {
+            self
+        };
+    }
+
+    /// Returns the contents before the last occurrence of `sub` in the string.
+    public func all_before_of_last(self, sub: Self) -> Self {
+        return if pos := self.last_index_of(sub) {
+            self[..pos]
+        } else {
+            self
+        };
+    }
+
+    /// Returns the contents after `sub` in the string. If the substring is
+    /// not found, it returns the full input string.
+    public func all_after_of(self, sub: Self) -> Self {
+        return if pos := self.index_of(sub) {
+            self[pos + sub.len..]
+        } else {
+            self
+        };
+    }
+
+    /// Returns the contents after the last occurrence of `sub` in the string.
+    /// If the substring is not found, it returns the full input string.
+    public func all_after_of_last(self, sub: Self) -> Self {
+        return if pos := self.last_index_of(sub) {
+            self[pos + sub.len..]
+        } else {
+            self
+        };
     }
 
     /// Returns the index of byte `b` if found in the string.
@@ -281,11 +311,16 @@ public struct string < Stringable, Hashable, Throwable {
 
     /// Linear search for the last index of `byte` inside a string.
     public func last_index_of_byte(self, byte: uint8) -> ?uint {
-        mut i: uint := self.len;
-        while i != 0 {
-            i -= 1;
+        mut i: uint := self.len - 1;
+        while i >= 0 : i -= 1 {
             if unsafe { self.ptr[i] == byte } {
                 return i;
+            } else if i == 0 {
+                // avoid panic because the value of `i` will reach 0, then `1`
+                // will be subtracted from it, which will cause an integer
+                // overflow and cause `i` to be equal to `uint.MAX`.
+                // TODO: find a way to avoid this in code generation.
+                break;
             }
         }
         return none;
@@ -303,6 +338,9 @@ public struct string < Stringable, Hashable, Throwable {
             if j == p.len {
                 return i;
             }
+            if i == 0 {
+                break;
+            }
         }
         return none;
     }
@@ -398,6 +436,9 @@ public struct string < Stringable, Hashable, Throwable {
             if !found {
                 break;
             }
+            if pos == 0 {
+                break;
+            }
         }
         return self.slice(0, pos + 1);
     }

lib/rivet/src/ast/Table.ri

@@ -386,12 +386,12 @@ public struct Table {
     public func filter_files(self, inputs: []string) -> []string {
         mut new_inputs := @vec(string, inputs.len);
         for input in inputs {
-            basename_input := Path.basename(input);
-            if basename_input.count(".") == 1 {
+            base_name_input := Path.base_name(input);
+            if base_name_input.count(".") == 1 {
                 new_inputs.push(input);
                 continue;
             }
-            exts := basename_input[..basename_input.len - 3].split(".")[1..];
+            exts := base_name_input[..base_name_input.len - 3].split(".")[1..];
             mut already_exts := @vec(string, exts.len);
             mut should_compile := false;
             for ext in exts {

lib/rivet/src/lib.ri

@@ -179,13 +179,13 @@ public struct Compiler {
             } else {
                 pathx2
             };
-            dirname := Path.resolve(Path.dirname(file_path) ?? file_path)!;
+            dir_name := Path.resolve(Path.dir_name(file_path) ?? file_path)!;
             old_wd := process.get_cwd()!;
-            process.set_cwd(dirname)!;
+            process.set_cwd(dir_name)!;
             if Path.is_directory(pathx) {
                 found = true;
                 abspath = Path.resolve(pathx)!;
-                mut mod_basedir := Path.dirname(abspath) ?? abspath;
+                mut mod_basedir := Path.dir_name(abspath) ?? abspath;
                 if mod_basedir.ends_with("/src") {
                     mod_basedir = mod_basedir[..mod_basedir.len - 4]; // skip `/src`
                 } else if mod_basedir.contains("/src") {

lib/rivet/src/parser/mod.ri

@@ -48,7 +48,7 @@ public struct Parser {
 
     public func parse_file(mut self, file: string) -> ast.SourceFile {
         self.file_path = file;
-        self.file_dir = Path.dirname(file) ?? file;
+        self.file_dir = Path.dir_name(file) ?? file;
         self.tokenizer = tokenizer.Tokenizer.from_file(file, self.prefs, self.table);
         if report.total_errors() > 0 {
             return ast.SourceFile(file, [], self.mod_sym);

lib/rivet/src/prefs/mod.ri

@@ -11,7 +11,7 @@ import { Path } from std/fs;
 import ../utils;
 
 public static rivetDir := Path.join(env.homeDir, ".rivet_lang")!;
-public static rivetcDir := Path.dirname(process.executable()!)?;
+public static rivetcDir := Path.dir_name(process.executable()!)?;
 
 public static objDir := Path.join(rivetDir, "obj")!;
 public static libDir := Path.join(rivetDir, "lib")!;
@@ -281,9 +281,9 @@ public struct Prefs {
                 prefs.input = arg;
                 if prefs.mod_name.is_empty() {
                     prefs.mod_name = if Path.is_file(arg) {
-                        Path.basename(arg).all_before_of(".ri")
+                        Path.base_name(arg).all_before_of(".ri")
                     } else {
-                        Path.basename(Path.resolve(arg) catch arg)
+                        Path.base_name(Path.resolve(arg) catch arg)
                     };
                 }
             }
@@ -308,7 +308,7 @@ public struct Prefs {
         }
 
         prefs.mod_dir = if Path.is_file(prefs.input) {
-            Path.dirname(Path.absolute(prefs.input)!) ?? prefs.input
+            Path.dir_name(Path.absolute(prefs.input)!) ?? prefs.input
         } else {
             prefs.input
         };

lib/std/src/fs/Path.ri

@@ -17,7 +17,7 @@ public struct Path {
     /// Returns `true` if the given byte is a valid path separator
     #[inline]
     public func is_separator(byte: uint8) -> bool {
-        return byte == b'/' #if _WINDOWS_ or byte == b'\\' #endif;
+        return byte == SEPARATOR;
     }
 
     /// Returns `true` if `path` (file or directory) exists.
@@ -81,50 +81,75 @@ public struct Path {
     /// If the path is a file in the current directory (no directory component)
     /// then returns `.`.
     /// If the path is the root directory, returns `/`.
-    public func dirname(path: string) -> ?string {
+    public func dir_name(path: string) -> ?string {
         if path.is_empty() {
             return none;
         }
-        if pos := path.last_index_of_byte(SEPARATOR) {
-            if pos == 0 and SEPARATOR == b'/' {
-                return "/";
+
+        mut end_index: uint := path.len - 1;
+        while path[end_index] == SEPARATOR : end_index -= 1 {
+            if end_index == 0 {
+                return none;
+            }
+        }
+
+        while path[end_index] != SEPARATOR : end_index -= 1 {
+            if end_index == 0 {
+                return none;
             }
-            return path[..pos];
         }
-        return ".";
+
+        if end_index == 0 and path[0] == SEPARATOR {
+            return separatorStr;
+        } else if end_index == 0 {
+            return none;
+        }
+
+        return path[..end_index];
     }
 
-    public func basename(path: string) -> string {
+    /// Returns the last element of path. Trailing path separators are removed before
+    /// extracting the last element. If the path is empty, base returns ".". If the
+    /// path consists entirely of separators, base returns a single separator.
+    public func base_name(path: string) -> string {
         if path.is_empty() {
             return "";
         }
         mut end_index: uint := path.len - 1;
-        while path[end_index] == b'/' : end_index -= 1 {
+        while path[end_index] == SEPARATOR : end_index -= 1 {
             if end_index == 0 {
                 return "";
             }
         }
         mut start_index: uint := end_index;
         end_index += 1;
-        while path[start_index] != b'/' : start_index -= 1 {
+        while path[start_index] != SEPARATOR : start_index -= 1 {
             if start_index == 0 {
                 return path[0..end_index];
             }
         }
         return path[start_index + 1 .. end_index];
     }
 
+    /// Return alls characters found after the last occurrence of `separatorStr`.
+    /// File extension is included.
+    public func file_name(path: string) -> string {
+        return path.all_after_of_last(separatorStr);
+    }
+
     /// Returns the extension of the file name (if any).
     /// This function will search for the file extension (separated by a `.`) and will
     /// return the text after the `.`.
     /// Files that end with `.`, or that start with `.` and have no other `.` in their
     /// name, are considered to have no extension.
-    /// The returned slice is guaranteed to have its pointer within the start and end
-    /// pointer address range of `path`, even if it is length zero.
     public func extension(path: string) -> string {
-        filename := Self.basename(path);
-        index := filename.last_index_of_byte(b'.') ?? return path[path.len..];
-        return if index == 0 { path[path.len..] } else { filename[index..] };
+        filename := Self.file_name(path);
+        index := filename.last_index_of_byte(b'.') ?? return "";
+        return if index == 0 or index + 1 >= filename.len {
+            ""
+        } else {
+            filename[index..]
+        };
     }
 
     /// This function is like a series of `cd` statements executed one after another.

lib/std/src/process/mod.ri

@@ -17,7 +17,7 @@ public alias executable := core.process_executable;
 
 public static args := core.ARGS;
 public static wdAtStartup := get_cwd() catch ".";
-public static executableDir := Path.dirname(executable() catch wdAtStartup);
+public static executableDir := Path.dir_name(executable() catch wdAtStartup);
 
 #[boxed]
 public struct NotADirectoryError < Throwable {

lib/std/tests/fs_test.ri

@@ -38,52 +38,59 @@ test "std.fs.Path.resolve()" {
     );
 }
 
-test "std.fs.Path.dirname()" {
-    if d1 := Path.dirname("/a/b/c") {
+test "std.fs.Path.dir_name()" {
+    if d1 := Path.dir_name("/a/b/c") {
         @assert(d1 == "/a/b");
     } else {
         @assert(false);
     }
-    if d2 := Path.dirname("/a/b/c///") {
+    if d2 := Path.dir_name("/a/b/c///") {
         @assert(d2 == "/a/b");
     } else {
         @assert(false);
     }
-    if d3 := Path.dirname("/a") {
+    if d3 := Path.dir_name("/a") {
         @assert(d3 == "/");
     } else {
         @assert(false);
     }
-    @assert(Path.dirname("/") is none);
-    @assert(Path.dirname("//") is none);
-    @assert(Path.dirname("///") is none);
-    @assert(Path.dirname("////") is none);
-    @assert(Path.dirname("") is none);
-    @assert(Path.dirname("a") is none);
-    @assert(Path.dirname("a/") is none);
-    @assert(Path.dirname("a//") is none);
+    @assert(Path.dir_name("/") is none);
+    @assert(Path.dir_name("//") is none);
+    @assert(Path.dir_name("///") is none);
+    @assert(Path.dir_name("////") is none);
+    @assert(Path.dir_name("") is none);
+    @assert(Path.dir_name("a") is none);
+    @assert(Path.dir_name("a/") is none);
+    @assert(Path.dir_name("a//") is none);
 }
 
-test "std.fs.Path.basename()" {
-    @assert(Path.basename("") == "");
-    @assert(Path.basename("/") == "");
-    @assert(Path.basename("/dir/basename.ext") == "basename.ext");
-    @assert(Path.basename("/basename.ext") == "basename.ext");
-    @assert(Path.basename("basename.ext") == "basename.ext");
-    @assert(Path.basename("basename.ext/") == "basename.ext");
-    @assert(Path.basename("basename.ext//") == "basename.ext");
-    @assert(Path.basename("/aaa/bbb") == "bbb");
-    @assert(Path.basename("/aaa/") == "aaa");
-    @assert(Path.basename("/aaa/b") == "b");
-    @assert(Path.basename("/a/b") == "b");
-    @assert(Path.basename("//a") == "a");
+test "std.fs.Path.base_name()" {
+    @assert(Path.base_name("") == "");
+    @assert(Path.base_name("/") == "");
+    @assert(Path.base_name("/dir/base_name.ext") == "base_name.ext");
+    @assert(Path.base_name("/base_name.ext") == "base_name.ext");
+    @assert(Path.base_name("base_name.ext") == "base_name.ext");
+    @assert(Path.base_name("base_name.ext/") == "base_name.ext");
+    @assert(Path.base_name("base_name.ext//") == "base_name.ext");
+    @assert(Path.base_name("/aaa/bbb") == "bbb");
+    @assert(Path.base_name("/aaa/") == "aaa");
+    @assert(Path.base_name("/aaa/b") == "b");
+    @assert(Path.base_name("/a/b") == "b");
+    @assert(Path.base_name("//a") == "a");
 
-    @assert(Path.basename("\\dir\\basename.ext") == "\\dir\\basename.ext");
-    @assert(Path.basename("\\basename.ext") == "\\basename.ext");
-    @assert(Path.basename("basename.ext") == "basename.ext");
-    @assert(Path.basename("basename.ext\\") == "basename.ext\\");
-    @assert(Path.basename("basename.ext\\\\") == "basename.ext\\\\");
-    @assert(Path.basename("foo") == "foo");
+    @assert(Path.base_name("\\dir\\base_name.ext") == "\\dir\\base_name.ext");
+    @assert(Path.base_name("\\base_name.ext") == "\\base_name.ext");
+    @assert(Path.base_name("base_name.ext") == "base_name.ext");
+    @assert(Path.base_name("base_name.ext\\") == "base_name.ext\\");
+    @assert(Path.base_name("base_name.ext\\\\") == "base_name.ext\\\\");
+    @assert(Path.base_name("foo") == "foo");
+}
+
+test "std.fs.Path.file_name()" {
+    @assert(Path.file_name("rivet/lib/std/project.json") == "project.json");
+    @assert(Path.file_name("rivet/lib/std/") == "");
+    @assert(Path.file_name("rivet/lib/std") == "std");
+    @assert(Path.file_name("filename") == "filename");
 }
 
 test "std.fs.Path.extension()" {

rivetc/src/codegen/__init__.py

@@ -2051,7 +2051,7 @@ def gen_expr(self, expr, custom_tmp = None):
                 return ir.Ident(ir.BOOL_T, tmp)
 
             left = self.gen_expr_with_cast(expr_left_typ, expr.left)
-            right = self.gen_expr_with_cast(expr_right_typ, expr.right)
+            right = self.gen_expr_with_cast(expr_left_typ, expr.right)
 
             # runtime calculation
             tmp = self.cur_fn.local_name()