Browse Source

Add mutex to Scope lookups and insertions

gingerBill 4 years ago
parent
commit
a01c946c20
4 changed files with 55 additions and 43 deletions
  1. 2 0
      src/check_decl.cpp
  2. 39 30
      src/checker.cpp
  3. 1 0
      src/checker.hpp
  4. 13 13
      src/parser.cpp

+ 2 - 0
src/check_decl.cpp

@@ -334,6 +334,8 @@ void override_entity_in_scope(Entity *original_entity, Entity *new_entity) {
 	if (found_scope == nullptr) {
 	if (found_scope == nullptr) {
 		return;
 		return;
 	}
 	}
+	mutex_lock(&found_scope->mutex);
+	defer (mutex_unlock(&found_scope->mutex));
 
 
 	// IMPORTANT NOTE(bill, 2021-04-10): Overriding behaviour was flawed in that the
 	// IMPORTANT NOTE(bill, 2021-04-10): Overriding behaviour was flawed in that the
 	// original entity was still used check checked, but the checking was only
 	// original entity was still used check checked, but the checking was only

+ 39 - 30
src/checker.cpp

@@ -227,6 +227,7 @@ Scope *create_scope(CheckerInfo *info, Scope *parent, isize init_elements_capaci
 	s->parent = parent;
 	s->parent = parent;
 	string_map_init(&s->elements, heap_allocator(), init_elements_capacity);
 	string_map_init(&s->elements, heap_allocator(), init_elements_capacity);
 	ptr_set_init(&s->imported, heap_allocator(), 0);
 	ptr_set_init(&s->imported, heap_allocator(), 0);
+	mutex_init(&s->mutex);
 
 
 	if (parent != nullptr && parent != builtin_pkg->scope) {
 	if (parent != nullptr && parent != builtin_pkg->scope) {
 		Scope *prev_head_child = parent->head_child.exchange(s, std::memory_order_acq_rel);
 		Scope *prev_head_child = parent->head_child.exchange(s, std::memory_order_acq_rel);
@@ -307,6 +308,7 @@ void destroy_scope(Scope *scope) {
 
 
 	string_map_destroy(&scope->elements);
 	string_map_destroy(&scope->elements);
 	ptr_set_destroy(&scope->imported);
 	ptr_set_destroy(&scope->imported);
+	mutex_destroy(&scope->mutex);
 
 
 	// NOTE(bill): No need to free scope as it "should" be allocated in an arena (except for the global scope)
 	// NOTE(bill): No need to free scope as it "should" be allocated in an arena (except for the global scope)
 }
 }
@@ -356,43 +358,46 @@ Entity *scope_lookup_current(Scope *s, String const &name) {
 }
 }
 
 
 void scope_lookup_parent(Scope *scope, String const &name, Scope **scope_, Entity **entity_) {
 void scope_lookup_parent(Scope *scope, String const &name, Scope **scope_, Entity **entity_) {
-	bool gone_thru_proc = false;
-	bool gone_thru_package = false;
-	StringHashKey key = string_hash_string(name);
-	for (Scope *s = scope; s != nullptr; s = s->parent) {
-		Entity **found = string_map_get(&s->elements, key);
-		if (found) {
-			Entity *e = *found;
-			if (gone_thru_proc) {
-				// IMPORTANT TODO(bill): Is this correct?!
-				if (e->kind == Entity_Label) {
-					continue;
-				}
-				if (e->kind == Entity_Variable) {
-					if (e->scope->flags&ScopeFlag_File) {
-						// Global variables are file to access
-					} else if (e->flags&EntityFlag_Static) {
-						// Allow static/thread_local variables to be referenced
-					} else {
+	if (scope != nullptr) {
+		bool gone_thru_proc = false;
+		bool gone_thru_package = false;
+		StringHashKey key = string_hash_string(name);
+		for (Scope *s = scope; s != nullptr; s = s->parent) {
+			Entity **found = nullptr;
+			mutex_lock(&s->mutex);
+			found = string_map_get(&s->elements, key);
+			mutex_unlock(&s->mutex);
+			if (found) {
+				Entity *e = *found;
+				if (gone_thru_proc) {
+					// IMPORTANT TODO(bill): Is this correct?!
+					if (e->kind == Entity_Label) {
 						continue;
 						continue;
 					}
 					}
+					if (e->kind == Entity_Variable) {
+						if (e->scope->flags&ScopeFlag_File) {
+							// Global variables are file to access
+						} else if (e->flags&EntityFlag_Static) {
+							// Allow static/thread_local variables to be referenced
+						} else {
+							continue;
+						}
+					}
 				}
 				}
-			}
 
 
-			if (entity_) *entity_ = e;
-			if (scope_) *scope_ = s;
-			return;
-		}
+				if (entity_) *entity_ = e;
+				if (scope_) *scope_ = s;
+				return;
+			}
 
 
-		if (s->flags&ScopeFlag_Proc) {
-			gone_thru_proc = true;
-		}
-		if (s->flags&ScopeFlag_Pkg) {
-			gone_thru_package = true;
+			if (s->flags&ScopeFlag_Proc) {
+				gone_thru_proc = true;
+			}
+			if (s->flags&ScopeFlag_Pkg) {
+				gone_thru_package = true;
+			}
 		}
 		}
 	}
 	}
-
-
 	if (entity_) *entity_ = nullptr;
 	if (entity_) *entity_ = nullptr;
 	if (scope_) *scope_ = nullptr;
 	if (scope_) *scope_ = nullptr;
 }
 }
@@ -410,6 +415,10 @@ Entity *scope_insert_with_name(Scope *s, String const &name, Entity *entity) {
 		return nullptr;
 		return nullptr;
 	}
 	}
 	StringHashKey key = string_hash_string(name);
 	StringHashKey key = string_hash_string(name);
+
+	mutex_lock(&s->mutex);
+	defer (mutex_unlock(&s->mutex));
+
 	Entity **found = string_map_get(&s->elements, key);
 	Entity **found = string_map_get(&s->elements, key);
 
 
 	if (found) {
 	if (found) {

+ 1 - 0
src/checker.hpp

@@ -191,6 +191,7 @@ struct Scope {
 	std::atomic<Scope *> next;
 	std::atomic<Scope *> next;
 	std::atomic<Scope *> head_child;
 	std::atomic<Scope *> head_child;
 
 
+	BlockingMutex mutex;
 	StringMap<Entity *> elements;
 	StringMap<Entity *> elements;
 	PtrSet<Scope *> imported;
 	PtrSet<Scope *> imported;
 
 

+ 13 - 13
src/parser.cpp

@@ -4868,7 +4868,7 @@ void parser_add_package(Parser *p, AstPackage *pkg) {
 	}
 	}
 }
 }
 
 
-ParseFileError process_imported_file(Parser *p, ImportedFile const &imported_file);
+ParseFileError process_imported_file(Parser *p, ImportedFile imported_file);
 
 
 WORKER_TASK_PROC(parser_worker_proc) {
 WORKER_TASK_PROC(parser_worker_proc) {
 	ParserWorkerData *wd = cast(ParserWorkerData *)data;
 	ParserWorkerData *wd = cast(ParserWorkerData *)data;
@@ -5543,47 +5543,47 @@ bool parse_file(Parser *p, AstFile *f) {
 }
 }
 
 
 
 
-ParseFileError process_imported_file(Parser *p, ImportedFile const &imported_file) {
+ParseFileError process_imported_file(Parser *p, ImportedFile imported_file) {
 	AstPackage *pkg = imported_file.pkg;
 	AstPackage *pkg = imported_file.pkg;
-	FileInfo const *fi = &imported_file.fi;
-	TokenPos pos = imported_file.pos;
+	FileInfo    fi  = imported_file.fi;
+	TokenPos    pos = imported_file.pos;
 
 
 	AstFile *file = gb_alloc_item(heap_allocator(), AstFile);
 	AstFile *file = gb_alloc_item(heap_allocator(), AstFile);
 	file->pkg = pkg;
 	file->pkg = pkg;
 	file->id = cast(i32)(imported_file.index+1);
 	file->id = cast(i32)(imported_file.index+1);
 	TokenPos err_pos = {0};
 	TokenPos err_pos = {0};
-	ParseFileError err = init_ast_file(file, fi->fullpath, &err_pos);
+	ParseFileError err = init_ast_file(file, fi.fullpath, &err_pos);
 	err_pos.file_id = file->id;
 	err_pos.file_id = file->id;
 	file->last_error = err;
 	file->last_error = err;
 
 
 	if (err != ParseFile_None) {
 	if (err != ParseFile_None) {
 		if (err == ParseFile_EmptyFile) {
 		if (err == ParseFile_EmptyFile) {
-			if (fi->fullpath == p->init_fullpath) {
+			if (fi.fullpath == p->init_fullpath) {
 				syntax_error(pos, "Initial file is empty - %.*s\n", LIT(p->init_fullpath));
 				syntax_error(pos, "Initial file is empty - %.*s\n", LIT(p->init_fullpath));
 				gb_exit(1);
 				gb_exit(1);
 			}
 			}
 		} else {
 		} else {
 			switch (err) {
 			switch (err) {
 			case ParseFile_WrongExtension:
 			case ParseFile_WrongExtension:
-				syntax_error(pos, "Failed to parse file: %.*s; invalid file extension: File must have the extension '.odin'", LIT(fi->name));
+				syntax_error(pos, "Failed to parse file: %.*s; invalid file extension: File must have the extension '.odin'", LIT(fi.name));
 				break;
 				break;
 			case ParseFile_InvalidFile:
 			case ParseFile_InvalidFile:
-				syntax_error(pos, "Failed to parse file: %.*s; invalid file or cannot be found", LIT(fi->name));
+				syntax_error(pos, "Failed to parse file: %.*s; invalid file or cannot be found", LIT(fi.name));
 				break;
 				break;
 			case ParseFile_Permission:
 			case ParseFile_Permission:
-				syntax_error(pos, "Failed to parse file: %.*s; file permissions problem", LIT(fi->name));
+				syntax_error(pos, "Failed to parse file: %.*s; file permissions problem", LIT(fi.name));
 				break;
 				break;
 			case ParseFile_NotFound:
 			case ParseFile_NotFound:
-				syntax_error(pos, "Failed to parse file: %.*s; file cannot be found ('%.*s')", LIT(fi->name), LIT(fi->fullpath));
+				syntax_error(pos, "Failed to parse file: %.*s; file cannot be found ('%.*s')", LIT(fi.name), LIT(fi.fullpath));
 				break;
 				break;
 			case ParseFile_InvalidToken:
 			case ParseFile_InvalidToken:
-				syntax_error(err_pos, "Failed to parse file: %.*s; invalid token found in file", LIT(fi->name));
+				syntax_error(err_pos, "Failed to parse file: %.*s; invalid token found in file", LIT(fi.name));
 				break;
 				break;
 			case ParseFile_EmptyFile:
 			case ParseFile_EmptyFile:
-				syntax_error(pos, "Failed to parse file: %.*s; file contains no tokens", LIT(fi->name));
+				syntax_error(pos, "Failed to parse file: %.*s; file contains no tokens", LIT(fi.name));
 				break;
 				break;
 			case ParseFile_FileTooLarge:
 			case ParseFile_FileTooLarge:
-				syntax_error(pos, "Failed to parse file: %.*s; file is too large, exceeds maximum file size of 2 GiB", LIT(fi->name));
+				syntax_error(pos, "Failed to parse file: %.*s; file is too large, exceeds maximum file size of 2 GiB", LIT(fi.name));
 				break;
 				break;
 			}
 			}