From fe4d819a865b92118584ee5fbc0609e59ad8b2fb Mon Sep 17 00:00:00 2001 From: Daniel Gavin Date: Sun, 23 Oct 2022 15:45:01 +0200 Subject: Try to use recursion map on pointers to prevent stackoverflow crashes --- .gitignore | 1 + editors/vscode/package.json | 2 +- editors/vscode/syntaxes/odin.tmLanguage.json | 2 +- ols.json | 9 +- src/main.odin | 11 +- src/server/analysis.odin | 214 ++++++++++++++++----------- src/server/completion.odin | 29 ++-- src/server/hover.odin | 27 ++-- src/server/references.odin | 9 +- 9 files changed, 171 insertions(+), 133 deletions(-) diff --git a/.gitignore b/.gitignore index 2a136bc..fa9ecb8 100644 --- a/.gitignore +++ b/.gitignore @@ -10,4 +10,5 @@ *.exp *.sqlite *.suo +*node_modules diff --git a/editors/vscode/package.json b/editors/vscode/package.json index a17b6a6..cf4fb6d 100644 --- a/editors/vscode/package.json +++ b/editors/vscode/package.json @@ -7,7 +7,7 @@ "type": "git", "url": "git://github.com/DanielGavin/ols.git" }, - "version": "0.1.4", + "version": "0.1.6", "engines": { "vscode": "^1.65.0" }, diff --git a/editors/vscode/syntaxes/odin.tmLanguage.json b/editors/vscode/syntaxes/odin.tmLanguage.json index d155e2d..a3d2cf3 100644 --- a/editors/vscode/syntaxes/odin.tmLanguage.json +++ b/editors/vscode/syntaxes/odin.tmLanguage.json @@ -90,7 +90,7 @@ }, { "name": "keyword.control.odin", - "match": "\\b(if|else|or_else|when|for|in|defer|switch|return|or_return)\\b" + "match": "\\b(if|else|or_else|when|for|in|not_in|defer|switch|return|or_return)\\b" }, { "name": "keyword.control.odin", diff --git a/ols.json b/ols.json index b56b6b1..5261a9c 100644 --- a/ols.json +++ b/ols.json @@ -1,9 +1,5 @@ { "collections": [ - { - "name": "core", - "path": "C:\\Users\\danie\\OneDrive\\Desktop\\Computer_Science\\Odin\\core" - }, { "name": "shared", "path": "src" @@ -11,6 +7,7 @@ ], "enable_document_symbols": true, "enable_semantic_tokens": true, - "enable_hover": true, - "enable_snippets": true + "enable_snippets": true, + "enable_references": true, + "verbose": false } diff --git a/src/main.odin b/src/main.odin index 3842c58..7823828 100644 --- a/src/main.odin +++ b/src/main.odin @@ -95,13 +95,11 @@ run :: proc(reader: ^server.Reader, writer: ^server.Writer) { end :: proc() { } - main :: proc() { reader := server.make_reader(os_read, cast(rawptr)&os.stdin) writer := server.make_writer(os_write, cast(rawptr)&os.stdout) - /* fh, err := os.open("log.txt", os.O_RDWR|os.O_CREATE) @@ -116,14 +114,7 @@ main :: proc() { set_stacktrace() } - when ODIN_OS == .Darwin { - init_global_temporary_allocator(mem.Megabyte * 100) - } else { - init_global_temporary_allocator(mem.Megabyte * 100) - //Gives weird allocation errors - //growing_arena: common.Growing_Arena - //context.temp_allocator = common.growing_arena_allocator(&growing_arena) - } + init_global_temporary_allocator(mem.Megabyte * 100) run(&reader, &writer) diff --git a/src/server/analysis.odin b/src/server/analysis.odin index c7081c5..2630b79 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -70,28 +70,28 @@ DocumentLocal :: struct { } AstContext :: struct { - locals: map[int]map[string][dynamic]DocumentLocal, //locals all the way to the document position - globals: map[string]common.GlobalExpr, - variables: map[string]bool, - parameters: map[string]bool, - in_package: map[string]string, //sometimes you have to extract types from arrays/maps and you lose package information - usings: [dynamic]string, - file: ast.File, - allocator: mem.Allocator, - imports: []Package, //imports for the current document - current_package: string, - document_package: string, - use_globals: bool, - use_locals: bool, - local_id: int, - call: ^ast.Call_Expr, //used to determene the types for generics and the correct function for overloaded functions - position: common.AbsolutePosition, - value_decl: ^ast.Value_Decl, - field_name: ast.Ident, - uri: string, - fullpath: string, - recursion_counter: int, //Sometimes the ast is so malformed that it causes infinite recursion. - non_mutable_only: bool, + locals: map[int]map[string][dynamic]DocumentLocal, //locals all the way to the document position + globals: map[string]common.GlobalExpr, + variables: map[string]bool, + parameters: map[string]bool, + in_package: map[string]string, //sometimes you have to extract types from arrays/maps and you lose package information + recursion_map: map[rawptr]bool, + usings: [dynamic]string, + file: ast.File, + allocator: mem.Allocator, + imports: []Package, //imports for the current document + current_package: string, + document_package: string, + use_globals: bool, + use_locals: bool, + local_id: int, + call: ^ast.Call_Expr, //used to determene the types for generics and the correct function for overloaded functions + position: common.AbsolutePosition, + value_decl: ^ast.Value_Decl, + field_name: ast.Ident, + uri: string, + fullpath: string, + non_mutable_only: bool, } make_ast_context :: proc( @@ -113,6 +113,7 @@ make_ast_context :: proc( usings = make([dynamic]string, allocator), parameters = make(map[string]bool, 0, allocator), in_package = make(map[string]string, 0, allocator), + recursion_map = make(map[rawptr]bool, 0, allocator), file = file, imports = imports, use_locals = true, @@ -129,6 +130,12 @@ make_ast_context :: proc( return ast_context } +reset_ast_context :: proc(ast_context: ^AstContext) { + ast_context.use_globals = true + ast_context.use_locals = true + clear(&ast_context.recursion_map) +} + tokenizer_error_handler :: proc(pos: tokenizer.Pos, msg: string, args: ..any) { } @@ -1029,12 +1036,26 @@ resolve_basic_directive :: proc( ) ident.name = "Source_Code_Location" ast_context.current_package = ast_context.document_package - return resolve_type_identifier(ast_context, ident^) + return internal_resolve_type_identifier(ast_context, ident^) } return {}, false } +check_node_recursion :: proc( + ast_context: ^AstContext, + node: ^ast.Node, +) -> bool { + raw := cast(rawptr)node + + if raw in ast_context.recursion_map { + return true + } + + ast_context.recursion_map[raw] = true + + return false +} resolve_type_expression :: proc( ast_context: ^AstContext, @@ -1042,6 +1063,17 @@ resolve_type_expression :: proc( ) -> ( Symbol, bool, +) { + clear(&ast_context.recursion_map) + return internal_resolve_type_expression(ast_context, node) +} + +internal_resolve_type_expression :: proc( + ast_context: ^AstContext, + node: ^ast.Expr, +) -> ( + Symbol, + bool, ) { if node == nil { return {}, false @@ -1053,25 +1085,19 @@ resolve_type_expression :: proc( ast_context.current_package = saved_package } - if ast_context.recursion_counter > 200 { - log.error("Recursion passed 200 attempts - giving up", node) + if check_node_recursion(ast_context, node) { + log.error("Recursion detected") return {}, false } - ast_context.recursion_counter += 1 - - defer { - ast_context.recursion_counter -= 1 - } - using ast #partial switch v in node.derived { case ^ast.Value_Decl: if v.type != nil { - return resolve_type_expression(ast_context, v.type) + return internal_resolve_type_expression(ast_context, v.type) } else if len(v.values) > 0 { - return resolve_type_expression(ast_context, v.values[0]) + return internal_resolve_type_expression(ast_context, v.values[0]) } case ^Union_Type: return make_symbol_union_from_ast( @@ -1150,47 +1176,49 @@ resolve_type_expression :: proc( case ^Basic_Lit: return resolve_basic_lit(ast_context, v^) case ^Type_Cast: - return resolve_type_expression(ast_context, v.type) + return internal_resolve_type_expression(ast_context, v.type) case ^Auto_Cast: - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Comp_Lit: - return resolve_type_expression(ast_context, v.type) + return internal_resolve_type_expression(ast_context, v.type) case ^Unary_Expr: if v.op.kind == .And { - symbol, ok := resolve_type_expression(ast_context, v.expr) + symbol, ok := internal_resolve_type_expression(ast_context, v.expr) symbol.pointers += 1 return symbol, ok } else { - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) } case ^Deref_Expr: - symbol, ok := resolve_type_expression(ast_context, v.expr) + symbol, ok := internal_resolve_type_expression(ast_context, v.expr) symbol.pointers -= 1 return symbol, ok case ^Paren_Expr: - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Slice_Expr: - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Tag_Expr: - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Helper_Type: - return resolve_type_expression(ast_context, v.type) + return internal_resolve_type_expression(ast_context, v.type) case ^Ellipsis: - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Implicit: ident := new_type(Ident, v.node.pos, v.node.end, ast_context.allocator) ident.name = v.tok.text - return resolve_type_identifier(ast_context, ident^) + return internal_resolve_type_identifier(ast_context, ident^) case ^Type_Assertion: if unary, ok := v.type.derived.(^ast.Unary_Expr); ok { if unary.op.kind == .Question { - if symbol, ok := resolve_type_expression(ast_context, v.expr); - ok { + if symbol, ok := internal_resolve_type_expression( + ast_context, + v.expr, + ); ok { if union_value, ok := symbol.value.(SymbolUnionValue); ok { if len(union_value.types) != 1 { return {}, false } - return resolve_type_expression( + return internal_resolve_type_expression( ast_context, union_value.types[0], ) @@ -1198,23 +1226,23 @@ resolve_type_expression :: proc( } } } else { - return resolve_type_expression(ast_context, v.type) + return internal_resolve_type_expression(ast_context, v.type) } case ^Proc_Lit: if v.type.results != nil { if len(v.type.results.list) == 1 { - return resolve_type_expression( + return internal_resolve_type_expression( ast_context, v.type.results.list[0].type, ) } } case ^Pointer_Type: - symbol, ok := resolve_type_expression(ast_context, v.elem) + symbol, ok := internal_resolve_type_expression(ast_context, v.elem) symbol.pointers += 1 return symbol, ok case ^Index_Expr: - indexed, ok := resolve_type_expression(ast_context, v.expr) + indexed, ok := internal_resolve_type_expression(ast_context, v.expr) if !ok { return {}, false @@ -1224,15 +1252,18 @@ resolve_type_expression :: proc( #partial switch v2 in indexed.value { case SymbolDynamicArrayValue: - symbol, ok = resolve_type_expression(ast_context, v2.expr) + symbol, ok = internal_resolve_type_expression(ast_context, v2.expr) case SymbolSliceValue: - symbol, ok = resolve_type_expression(ast_context, v2.expr) + symbol, ok = internal_resolve_type_expression(ast_context, v2.expr) case SymbolFixedArrayValue: - symbol, ok = resolve_type_expression(ast_context, v2.expr) + symbol, ok = internal_resolve_type_expression(ast_context, v2.expr) case SymbolMapValue: - symbol, ok = resolve_type_expression(ast_context, v2.value) + symbol, ok = internal_resolve_type_expression( + ast_context, + v2.value, + ) case SymbolMultiPointer: - symbol, ok = resolve_type_expression(ast_context, v2.expr) + symbol, ok = internal_resolve_type_expression(ast_context, v2.expr) } symbol.type = indexed.type @@ -1240,13 +1271,16 @@ resolve_type_expression :: proc( return symbol, ok case ^Call_Expr: ast_context.call = cast(^Call_Expr)node - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Implicit_Selector_Expr: return Symbol{}, false case ^Selector_Call_Expr: - return resolve_type_expression(ast_context, v.expr) + return internal_resolve_type_expression(ast_context, v.expr) case ^Selector_Expr: - if selector, ok := resolve_type_expression(ast_context, v.expr); ok { + if selector, ok := internal_resolve_type_expression( + ast_context, + v.expr, + ); ok { ast_context.use_locals = false #partial switch s in selector.value { @@ -1276,7 +1310,10 @@ resolve_type_expression :: proc( ast_context.current_package = ast_context.document_package } - symbol, ok := resolve_type_expression(ast_context, s.expr) + symbol, ok := internal_resolve_type_expression( + ast_context, + s.expr, + ) symbol.type = .Variable return symbol, ok } else { @@ -1303,7 +1340,10 @@ resolve_type_expression :: proc( ) selector_expr.expr = s.return_types[0].type selector_expr.field = v.field - return resolve_type_expression(ast_context, selector_expr) + return internal_resolve_type_expression( + ast_context, + selector_expr, + ) } case SymbolStructValue: if selector.pkg != "" { @@ -1315,7 +1355,7 @@ resolve_type_expression :: proc( for name, i in s.names { if v.field != nil && name == v.field.name { ast_context.field_name = v.field^ - symbol, ok := resolve_type_expression( + symbol, ok := internal_resolve_type_expression( ast_context, s.types[i], ) @@ -1341,7 +1381,7 @@ resolve_type_expression :: proc( return Symbol{}, false } case: - log.warnf("default node kind, resolve_type_expression: %T", v) + log.warnf("default node kind, internal_resolve_type_expression: %T", v) if v == nil { return {}, false } @@ -1473,11 +1513,22 @@ resolve_type_identifier :: proc( ) -> ( Symbol, bool, +) { + clear(&ast_context.recursion_map) + return internal_resolve_type_identifier(ast_context, node) +} + +internal_resolve_type_identifier :: proc( + ast_context: ^AstContext, + node: ast.Ident, +) -> ( + Symbol, + bool, ) { using ast - if ast_context.recursion_counter > 200 { - log.error("Recursion passed 200 attempts - giving up", node) + if check_node_recursion(ast_context, node.derived.(^ast.Ident)) { + log.error("Recursion detected") return {}, false } @@ -1487,12 +1538,6 @@ resolve_type_identifier :: proc( ast_context.current_package = saved_package } - ast_context.recursion_counter += 1 - - defer { - ast_context.recursion_counter -= 1 - } - if pkg, ok := ast_context.in_package[node.name]; ok { ast_context.current_package = pkg } @@ -1561,7 +1606,10 @@ resolve_type_identifier :: proc( #partial switch v in local.derived { case ^Ident: - return_symbol, ok = resolve_type_identifier(ast_context, v^) + return_symbol, ok = internal_resolve_type_identifier( + ast_context, + v^, + ) case ^Union_Type: return_symbol, ok = make_symbol_union_from_ast(ast_context, v^, node), true @@ -1655,7 +1703,10 @@ resolve_type_identifier :: proc( #partial switch v in global.expr.derived { case ^Ident: - return_symbol, ok = resolve_type_identifier(ast_context, v^) + return_symbol, ok = internal_resolve_type_identifier( + ast_context, + v^, + ) case ^Struct_Type: return_symbol, ok = make_symbol_struct_from_ast(ast_context, v^, node), true @@ -1713,7 +1764,7 @@ resolve_type_identifier :: proc( return_symbol.name = node.name return_symbol.type = global.mutable ? .Variable : .Constant case: - return_symbol, ok = resolve_type_expression( + return_symbol, ok = internal_resolve_type_expression( ast_context, global.expr, ) @@ -2013,8 +2064,7 @@ resolve_location_selector :: proc( Symbol, bool, ) { - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) ast_context.current_package = ast_context.document_package symbol, ok := resolve_type_expression(ast_context, selector.expr) @@ -2657,8 +2707,7 @@ get_generic_assignment :: proc( ) { using ast - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) #partial switch v in value.derived { case ^Or_Return_Expr: @@ -2788,8 +2837,7 @@ get_locals_stmt :: proc( document_position: ^DocumentPositionContext, save_assign := false, ) { - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) ast_context.current_package = ast_context.document_package using ast @@ -3420,8 +3468,8 @@ resolve_entire_decl :: proc( } data := cast(^Visit_Data)visitor.data ast_context := data.ast_context - ast_context.use_locals = true - ast_context.use_globals = true + + reset_ast_context(ast_context) data.last_visit = node diff --git a/src/server/completion.odin b/src/server/completion.odin index bb29bfb..91d8231 100644 --- a/src/server/completion.odin +++ b/src/server/completion.odin @@ -101,10 +101,15 @@ get_completion_list :: proc( completion_type = .Package } - if position_context.switch_type_stmt != nil && - position_context.case_clause != nil && - position_context.switch_type_stmt.pos.offset > - position_context.switch_stmt.pos.offset { + done: if position_context.switch_type_stmt != nil && + position_context.case_clause != nil { + + if position_context.switch_stmt != nil && + position_context.switch_type_stmt.pos.offset > + position_context.switch_stmt.pos.offset { + break done + } + if assign, ok := position_context.switch_type_stmt.tag.derived.(^ast.Assign_Stmt); ok && assign.rhs != nil && len(assign.rhs) == 1 { ast_context.use_globals = true @@ -270,8 +275,7 @@ get_selector_completion :: proc( selector: Symbol ok: bool - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) selector, ok = resolve_type_expression( ast_context, @@ -604,8 +608,7 @@ get_implicit_completion :: proc( selector: Symbol - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) if selector.pkg != "" { ast_context.current_package = selector.pkg @@ -1080,8 +1083,7 @@ get_identifier_completion :: proc( } } - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) ast_context.current_package = ast_context.document_package ident := new_type( @@ -1126,8 +1128,8 @@ get_identifier_completion :: proc( k, ) - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) + ast_context.current_package = ast_context.document_package ident := new_type( @@ -1438,8 +1440,7 @@ get_type_switch_completion :: proc( } } - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) if assign, ok := position_context.switch_type_stmt.tag.derived.(^ast.Assign_Stmt); ok && assign.rhs != nil && len(assign.rhs) == 1 { diff --git a/src/server/hover.odin b/src/server/hover.odin index 0b1661b..602258d 100644 --- a/src/server/hover.odin +++ b/src/server/hover.odin @@ -139,8 +139,9 @@ get_hover_information :: proc( position_context.identifier^, ast_context.file.src, ) - ast_context.use_locals = true - ast_context.use_globals = true + + reset_ast_context(&ast_context) + ast_context.current_package = ast_context.document_package //if the base selector is the client wants to go to. @@ -150,9 +151,9 @@ get_hover_information :: proc( if position_in_node(base, position_context.position) { if resolved, ok := resolve_type_identifier( - &ast_context, - ident, - ); ok { + &ast_context, + ident, + ); ok { resolved.signature = get_signature( &ast_context, ident, @@ -199,9 +200,9 @@ get_hover_information :: proc( for name, i in v.names { if name == field { if symbol, ok := resolve_type_expression( - &ast_context, - v.types[i], - ); ok { + &ast_context, + v.types[i], + ); ok { symbol.name = name //TODO refractor - never set symbol name after creation - change writer_hover_content symbol.pkg = selector.name symbol.signature = common.node_to_string(v.types[i]) @@ -218,9 +219,9 @@ get_hover_information :: proc( if ident, ok := position_context.field.derived.(^ast.Ident); ok { if symbol, ok := resolve_type_identifier( - &ast_context, - ident^, - ); ok { + &ast_context, + ident^, + ); ok { hover.contents = write_hover_content( &ast_context, symbol, @@ -231,8 +232,8 @@ get_hover_information :: proc( } } } else if position_context.identifier != nil { - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(&ast_context) + ast_context.current_package = ast_context.document_package ident := position_context.identifier.derived.(^ast.Ident)^ diff --git a/src/server/references.odin b/src/server/references.odin index d7ffb9e..de5b3cb 100644 --- a/src/server/references.odin +++ b/src/server/references.odin @@ -75,8 +75,7 @@ resolve_references :: proc( walk_directories, ) - ast_context.use_locals = true - ast_context.use_globals = true + reset_ast_context(ast_context) if position_context.struct_type != nil && position_in_struct_names( @@ -92,9 +91,9 @@ resolve_references :: proc( return {}, true } else if position_context.selector_expr != nil { if resolved, ok := resolve_type_expression( - ast_context, - position_context.selector, - ); ok { + ast_context, + position_context.selector, + ); ok { if _, is_package := resolved.value.(SymbolPackageValue); !is_package { return {}, true -- cgit v1.2.3