Fix `override_entity_in_scope `behaviour to correctly to report the changes upstream better

5 files changed, 62 insertions, 45 deletions

diff --git a/src/check_decl.cpp b/src/check_decl.cpp
index 51c0b6ee5..ea6c036cc 100644
--- a/src/check_decl.cpp
+++ b/src/check_decl.cpp
@@ -343,22 +343,36 @@ void override_entity_in_scope(Entity *original_entity, Entity *new_entity) {
 		return;
 	}
 
-	// IMPORTANT TODO(bill)
-	// Date: 2018-09-29
-	// This assert fails on `using import` if the name of the alias is the same. What should be the expected behaviour?
-	// Namespace collision or override? Overridding is the current behaviour
+	// 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
+	// relying on "constant" data such as the Entity.type and Entity.Constant.value
 	//
-	//     using import "foo"
-	//     bar :: foo.bar;
-
-	// GB_ASSERT_MSG(found_entity == original_entity, "%.*s == %.*s", LIT(found_entity->token.string), LIT(new_entity->token.string));
+	// Therefore two things can be done: the type can be assigned to state that it
+	// has been "evaluated" and the variant data can be copied across
 
 	string_map_set(&found_scope->elements, original_name, new_entity);
+
+	original_entity->type = new_entity->type;
+
+	if (original_entity->identifier == nullptr) {
+		original_entity->identifier = new_entity->identifier;
+	}
+	if (original_entity->identifier != nullptr &&
+	    original_entity->identifier->kind == Ast_Ident) {
+		original_entity->identifier->Ident.entity = new_entity;
+	}
+	original_entity->flags |= EntityFlag_Overridden;
+
+	// IMPORTANT NOTE(bill, 2021-04-10): copy only the variants
+	// This is most likely NEVER required, but it does not at all hurt to keep
+	isize offset = cast(u8 *)&original_entity->Dummy.start - cast(u8 *)original_entity;
+	isize size = gb_size_of(*original_entity) - offset;
+	gb_memmove(cast(u8 *)original_entity, cast(u8 *)new_entity, size);
 }
 
 
 
-void check_const_decl(CheckerContext *ctx, Entity *e, Ast *type_expr, Ast *init, Type *named_type) {
+void check_const_decl(CheckerContext *ctx, Entity *&e, Ast *type_expr, Ast *init, Type *named_type) {
 	GB_ASSERT(e->type == nullptr);
 	GB_ASSERT(e->kind == Entity_Constant);
 
@@ -374,6 +388,7 @@ void check_const_decl(CheckerContext *ctx, Entity *e, Ast *type_expr, Ast *init,
 
 	Operand operand = {};
 
+	Entity *other_entity = nullptr;
 	if (init != nullptr) {
 		Entity *entity = nullptr;
 		if (init->kind == Ast_Ident) {
@@ -412,7 +427,6 @@ void check_const_decl(CheckerContext *ctx, Entity *e, Ast *type_expr, Ast *init,
 			GB_ASSERT(operand.proc_group->kind == Entity_ProcGroup);
 			// NOTE(bill, 2020-06-10): It is better to just clone the contents than overriding the entity in the scope
 			// Thank goodness I made entities a tagged union to allow for this implace patching
-			// override_entity_in_scope(e, operand.proc_group);
 			e->kind = Entity_ProcGroup;
 			e->ProcGroup.entities = array_clone(heap_allocator(), operand.proc_group->ProcGroup.entities);
 			return;
@@ -454,7 +468,6 @@ void check_const_decl(CheckerContext *ctx, Entity *e, Ast *type_expr, Ast *init,
 							error(decl->attributes[0], "Constant alias declarations cannot have attributes");
 						}
 					}
-
 					return;
 				}
 			}
@@ -846,7 +859,7 @@ void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) {
 	}
 }
 
-void check_global_variable_decl(CheckerContext *ctx, Entity *e, Ast *type_expr, Ast *init_expr) {
+void check_global_variable_decl(CheckerContext *ctx, Entity *&e, Ast *type_expr, Ast *init_expr) {
 	GB_ASSERT(e->type == nullptr);
 	GB_ASSERT(e->kind == Entity_Variable);
 
@@ -946,7 +959,7 @@ void check_global_variable_decl(CheckerContext *ctx, Entity *e, Ast *type_expr,
 	check_init_variable(ctx, e, &o, str_lit("variable declaration"));
 }
 
-void check_proc_group_decl(CheckerContext *ctx, Entity *pg_entity, DeclInfo *d) {
+void check_proc_group_decl(CheckerContext *ctx, Entity *&pg_entity, DeclInfo *d) {
 	GB_ASSERT(pg_entity->kind == Entity_ProcGroup);
 	auto *pge = &pg_entity->ProcGroup;
 	String proc_group_name = pg_entity->token.string;
@@ -1074,7 +1087,7 @@ void check_proc_group_decl(CheckerContext *ctx, Entity *pg_entity, DeclInfo *d)
 
 }
 
-void check_entity_decl(CheckerContext *ctx, Entity *e, DeclInfo *d, Type *named_type) {
+void check_entity_decl(CheckerContext *ctx, Entity *&e, DeclInfo *d, Type *named_type) {
 	if (e->state == EntityState_Resolved)  {
 		return;
 	}
diff --git a/src/check_expr.cpp b/src/check_expr.cpp
index 2114746a3..8819ad6db 100644
--- a/src/check_expr.cpp
+++ b/src/check_expr.cpp
@@ -79,15 +79,12 @@ void     check_expr_with_type_hint      (CheckerContext *c, Operand *o, Ast *e,
 Type *   check_type                     (CheckerContext *c, Ast *expression);
 Type *   check_type_expr                (CheckerContext *c, Ast *expression, Type *named_type);
 Type *   make_optional_ok_type          (Type *value, bool typed=true);
-void     check_type_decl                (CheckerContext *c, Entity *e, Ast *type_expr, Type *def);
 Entity * check_selector                 (CheckerContext *c, Operand *operand, Ast *node, Type *type_hint);
 Entity * check_ident                    (CheckerContext *c, Operand *o, Ast *n, Type *named_type, Type *type_hint, bool allow_import_name);
 Entity * find_polymorphic_record_entity (CheckerContext *c, Type *original_type, isize param_count, Array<Operand> const &ordered_operands, bool *failure);
 void     check_not_tuple                (CheckerContext *c, Operand *operand);
 void     convert_to_typed               (CheckerContext *c, Operand *operand, Type *target_type);
 gbString expr_to_string                 (Ast *expression);
-void     check_entity_decl              (CheckerContext *c, Entity *e, DeclInfo *decl, Type *named_type);
-void     check_const_decl               (CheckerContext *c, Entity *e, Ast *type_expr, Ast *init_expr, Type *named_type);
 void     check_proc_body                (CheckerContext *c, Token token, DeclInfo *decl, Type *type, Ast *body);
 void     update_expr_type               (CheckerContext *c, Ast *e, Type *type, bool final);
 bool     check_is_terminating           (Ast *node, String const &label);
diff --git a/src/checker.cpp b/src/checker.cpp
index e0b303369..9233ad61e 100644
--- a/src/checker.cpp
+++ b/src/checker.cpp
@@ -2190,8 +2190,6 @@ Type *check_poly_path_pop(CheckerContext *c) {
 
 
 
-void check_entity_decl(CheckerContext *c, Entity *e, DeclInfo *d, Type *named_type);
-
 Array<Entity *> proc_group_entities(CheckerContext *c, Operand o) {
 	Array<Entity *> procs = {};
 	if (o.mode == Addressing_ProcGroup) {
diff --git a/src/checker.hpp b/src/checker.hpp
index b3e0b60ec..2ed79cb2f 100644
--- a/src/checker.hpp
+++ b/src/checker.hpp
@@ -396,6 +396,10 @@ void check_add_import_decl(CheckerContext *c, Ast *decl);
 void check_add_foreign_import_decl(CheckerContext *c, Ast *decl);
 
 
+void check_entity_decl(CheckerContext *c, Entity *&e, DeclInfo *d, Type *named_type);
+void check_const_decl(CheckerContext *c, Entity *&e, Ast *type_expr, Ast *init_expr, Type *named_type);
+void check_type_decl(CheckerContext *c, Entity *e, Ast *type_expr, Type *def);
+
 bool check_arity_match(CheckerContext *c, AstValueDecl *vd, bool is_global = false);
 void check_collect_entities(CheckerContext *c, Slice<Ast *> const &nodes);
 void check_collect_entities_from_when_stmt(CheckerContext *c, AstWhenStmt *ws);
diff --git a/src/entity.cpp b/src/entity.cpp
index 3926678fd..633576e25 100644
--- a/src/entity.cpp
+++ b/src/entity.cpp
@@ -33,39 +33,41 @@ String const entity_strings[] = {
 };
 
 enum EntityFlag : u64 {
-	EntityFlag_Visited       = 1<<0,
-	EntityFlag_Used          = 1<<1,
-	EntityFlag_Using         = 1<<2,
-	EntityFlag_Field         = 1<<3,
-	EntityFlag_Param         = 1<<4,
-	EntityFlag_Result        = 1<<5,
-	EntityFlag_ArrayElem     = 1<<6,
-	EntityFlag_Ellipsis      = 1<<7,
-	EntityFlag_NoAlias       = 1<<8,
-	EntityFlag_TypeField     = 1<<9,
-	EntityFlag_Value         = 1<<10,
-	EntityFlag_Sret          = 1<<11,
-	EntityFlag_ByVal         = 1<<12,
-	EntityFlag_BitFieldValue = 1<<13,
-	EntityFlag_PolyConst     = 1<<14,
-	EntityFlag_NotExported   = 1<<15,
-	EntityFlag_ConstInput    = 1<<16,
+	EntityFlag_Visited       = 1ull<<0,
+	EntityFlag_Used          = 1ull<<1,
+	EntityFlag_Using         = 1ull<<2,
+	EntityFlag_Field         = 1ull<<3,
+	EntityFlag_Param         = 1ull<<4,
+	EntityFlag_Result        = 1ull<<5,
+	EntityFlag_ArrayElem     = 1ull<<6,
+	EntityFlag_Ellipsis      = 1ull<<7,
+	EntityFlag_NoAlias       = 1ull<<8,
+	EntityFlag_TypeField     = 1ull<<9,
+	EntityFlag_Value         = 1ull<<10,
+	EntityFlag_Sret          = 1ull<<11,
+	EntityFlag_ByVal         = 1ull<<12,
+	EntityFlag_BitFieldValue = 1ull<<13,
+	EntityFlag_PolyConst     = 1ull<<14,
+	EntityFlag_NotExported   = 1ull<<15,
+	EntityFlag_ConstInput    = 1ull<<16,
 
-	EntityFlag_Static        = 1<<17,
+	EntityFlag_Static        = 1ull<<17,
 
-	EntityFlag_ImplicitReference = 1<<18, // NOTE(bill): equivalent to `const &` in C++
+	EntityFlag_ImplicitReference = 1ull<<18, // NOTE(bill): equivalent to `const &` in C++
 
-	EntityFlag_SoaPtrField   = 1<<19, // to allow s.x[0] where `s.x` is a pointer rather than a slice
+	EntityFlag_SoaPtrField   = 1ull<<19, // to allow s.x[0] where `s.x` is a pointer rather than a slice
 
-	EntityFlag_ProcBodyChecked = 1<<20,
+	EntityFlag_ProcBodyChecked = 1ull<<20,
 
-	EntityFlag_CVarArg       = 1<<21,
-	EntityFlag_AutoCast      = 1<<22,
+	EntityFlag_CVarArg       = 1ull<<21,
+	EntityFlag_AutoCast      = 1ull<<22,
 
-	EntityFlag_Disabled      = 1<<24,
-	EntityFlag_Cold          = 1<<25, // procedure is rarely called
+	EntityFlag_Disabled      = 1ull<<24,
+	EntityFlag_Cold          = 1ull<<25, // procedure is rarely called
 
-	EntityFlag_Test          = 1<<30,
+	EntityFlag_Test          = 1ull<<30,
+
+	EntityFlag_Overridden    = 1ull<<63,
 
 };
 
@@ -126,6 +128,9 @@ struct Entity {
 	// later entity kinds
 	union {
 		struct {
+			u8 start;
+		} Dummy;
+		struct {
 			ExactValue value;
 			ParameterValue param_value;
 			u32 flags;