Go through loads of TODOs

This commit is contained in:
gingerBill
2023-08-01 11:39:04 +01:00
parent 2f094134a3
commit 65206fe33e
13 changed files with 36 additions and 77 deletions
+7 -6
View File
@@ -7,13 +7,15 @@ gb_internal Type *check_init_variable(CheckerContext *ctx, Entity *e, Operand *o
e->type == t_invalid) {
if (operand->mode == Addressing_Builtin) {
ERROR_BLOCK();
gbString expr_str = expr_to_string(operand->expr);
// TODO(bill): is this a good enough error message?
error(operand->expr,
"Cannot assign built-in procedure '%s' in %.*s",
expr_str,
LIT(context_name));
"Cannot assign built-in procedure '%s' in %.*s",
expr_str,
LIT(context_name));
error_line("\tBuilt-in procedures are implemented by the compiler and might not be actually instantiated procedure\n");
operand->mode = Addressing_Invalid;
@@ -159,9 +161,8 @@ gb_internal void check_init_constant(CheckerContext *ctx, Entity *e, Operand *op
}
if (operand->mode != Addressing_Constant) {
// TODO(bill): better error
gbString str = expr_to_string(operand->expr);
error(operand->expr, "'%s' is not a constant", str);
error(operand->expr, "'%s' is not a compile-time known constant", str);
gb_string_free(str);
if (e->type == nullptr) {
e->type = t_invalid;
+3 -27
View File
@@ -462,7 +462,7 @@ gb_internal bool find_or_generate_polymorphic_procedure(CheckerContext *old_c, E
{
// LEAK TODO(bill): This is technically a memory leak as it has to generate the type twice
// LEAK NOTE(bill): This is technically a memory leak as it has to generate the type twice
bool prev_no_polymorphic_errors = nctx.no_polymorphic_errors;
defer (nctx.no_polymorphic_errors = prev_no_polymorphic_errors);
nctx.no_polymorphic_errors = false;
@@ -470,7 +470,7 @@ gb_internal bool find_or_generate_polymorphic_procedure(CheckerContext *old_c, E
// NOTE(bill): Reset scope from the failed procedure type
scope_reset(scope);
// LEAK TODO(bill): Cloning this AST may be leaky
// LEAK NOTE(bill): Cloning this AST may be leaky but this is not really an issue due to arena-based allocation
Ast *cloned_proc_type_node = clone_ast(pt->node);
success = check_procedure_type(&nctx, final_proc_type, cloned_proc_type_node, &operands);
if (!success) {
@@ -778,16 +778,6 @@ gb_internal i64 check_distance_between_types(CheckerContext *c, Operand *operand
}
}
// ^T <- rawptr
#if 0
// TODO(bill): Should C-style (not C++) pointer cast be allowed?
if (is_type_pointer(dst) && is_type_rawptr(src)) {
return true;
}
#endif
#if 1
// rawptr <- ^T
if (are_types_identical(type, t_rawptr) && is_type_pointer(src)) {
return 5;
@@ -808,7 +798,6 @@ gb_internal i64 check_distance_between_types(CheckerContext *c, Operand *operand
return 4;
}
}
#endif
if (is_type_polymorphic(dst) && !is_type_polymorphic(src)) {
bool modify_type = !c->no_polymorphic_errors;
@@ -824,7 +813,6 @@ gb_internal i64 check_distance_between_types(CheckerContext *c, Operand *operand
}
}
// TODO(bill): Determine which rule is a better on in practice
if (dst->Union.variants.count == 1) {
Type *vt = dst->Union.variants[0];
i64 score = check_distance_between_types(c, operand, vt);
@@ -1093,7 +1081,7 @@ gb_internal void check_assignment(CheckerContext *c, Operand *operand, Type *typ
// TODO(bill): is this a good enough error message?
error(operand->expr,
"Cannot assign overloaded procedure '%s' to '%s' in %.*s",
"Cannot assign overloaded procedure group '%s' to '%s' in %.*s",
expr_str,
op_type_str,
LIT(context_name));
@@ -1120,7 +1108,6 @@ gb_internal void check_assignment(CheckerContext *c, Operand *operand, Type *typ
switch (operand->mode) {
case Addressing_Builtin:
// TODO(bill): Actually allow built in procedures to be passed around and thus be created on use
error(operand->expr,
"Cannot assign built-in procedure '%s' in %.*s",
expr_str,
@@ -1412,9 +1399,6 @@ gb_internal bool is_polymorphic_type_assignable(CheckerContext *c, Type *poly, T
return false;
case Type_Proc:
if (source->kind == Type_Proc) {
// return check_is_assignable_to(c, &o, poly);
// TODO(bill): Polymorphic type assignment
#if 1
TypeProc *x = &poly->Proc;
TypeProc *y = &source->Proc;
if (x->calling_convention != y->calling_convention) {
@@ -1447,7 +1431,6 @@ gb_internal bool is_polymorphic_type_assignable(CheckerContext *c, Type *poly, T
}
return true;
#endif
}
return false;
case Type_Map:
@@ -1699,7 +1682,6 @@ gb_internal bool check_unary_op(CheckerContext *c, Operand *o, Token op) {
gb_string_free(str);
return false;
}
// TODO(bill): Handle errors correctly
Type *type = base_type(core_array_type(o->type));
gbString str = nullptr;
switch (op.kind) {
@@ -1743,7 +1725,6 @@ gb_internal bool check_unary_op(CheckerContext *c, Operand *o, Token op) {
gb_internal bool check_binary_op(CheckerContext *c, Operand *o, Token op) {
Type *main_type = o->type;
// TODO(bill): Handle errors correctly
Type *type = base_type(core_array_type(main_type));
Type *ct = core_type(type);
@@ -2775,8 +2756,6 @@ gb_internal void check_shift(CheckerContext *c, Operand *x, Operand *y, Ast *nod
gb_string_free(err_str);
}
// TODO(bill): Should we support shifts for fixed arrays and #simd vectors?
if (!is_type_integer(x->type)) {
gbString err_str = expr_to_string(x->expr);
error(node, "Shift operand '%s' must be an integer", err_str);
@@ -4437,7 +4416,6 @@ gb_internal ExactValue get_constant_field_single(CheckerContext *c, ExactValue v
case_end;
default:
// TODO(bill): Should this be a general fallback?
if (success_) *success_ = true;
if (finish_) *finish_ = true;
return empty_exact_value;
@@ -4793,8 +4771,6 @@ gb_internal Entity *check_selector(CheckerContext *c, Operand *operand, Ast *nod
}
if (entity == nullptr && selector->kind == Ast_Ident && is_type_array(type_deref(operand->type))) {
// TODO(bill): Simd_Vector swizzling
String field_name = selector->Ident.token.string;
if (1 < field_name.len && field_name.len <= 4) {
u8 swizzles_xyzw[4] = {'x', 'y', 'z', 'w'};
+5 -6
View File
@@ -384,7 +384,6 @@ gb_internal Type *check_assignment_variable(CheckerContext *ctx, Operand *lhs, O
}
if (e != nullptr) {
// HACK TODO(bill): Should the entities be freed as it's technically a leak
rhs->mode = Addressing_Value;
rhs->type = e->type;
rhs->proc_group = nullptr;
@@ -394,7 +393,7 @@ gb_internal Type *check_assignment_variable(CheckerContext *ctx, Operand *lhs, O
ast_node(i, Ident, node);
e = scope_lookup(ctx->scope, i->token.string);
if (e != nullptr && e->kind == Entity_Variable) {
used = (e->flags & EntityFlag_Used) != 0; // TODO(bill): Make backup just in case
used = (e->flags & EntityFlag_Used) != 0; // NOTE(bill): Make backup just in case
}
}
@@ -888,7 +887,7 @@ gb_internal void check_switch_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags
check_open_scope(ctx, node);
defer (check_close_scope(ctx));
check_label(ctx, ss->label, node); // TODO(bill): What should the label's "scope" be?
check_label(ctx, ss->label, node);
if (ss->init != nullptr) {
check_stmt(ctx, ss->init, 0);
@@ -1125,7 +1124,7 @@ gb_internal void check_type_switch_stmt(CheckerContext *ctx, Ast *node, u32 mod_
check_open_scope(ctx, node);
defer (check_close_scope(ctx));
check_label(ctx, ss->label, node); // TODO(bill): What should the label's "scope" be?
check_label(ctx, ss->label, node);
if (ss->tag->kind != Ast_AssignStmt) {
error(ss->tag, "Expected an 'in' assignment for this type switch statement");
@@ -1960,7 +1959,7 @@ gb_internal void check_value_decl_stmt(CheckerContext *ctx, Ast *node, u32 mod_f
Token token = ast_token(node);
if (vd->type != nullptr && entity_count > 1) {
error(token, "'using' can only be applied to one variable of the same type");
// TODO(bill): Should a 'continue' happen here?
// NOTE(bill): `using` will only be applied to a single declaration
}
for (isize entity_index = 0; entity_index < 1; entity_index++) {
@@ -2294,7 +2293,7 @@ gb_internal void check_for_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) {
mod_flags |= Stmt_BreakAllowed | Stmt_ContinueAllowed;
check_open_scope(ctx, node);
check_label(ctx, fs->label, node); // TODO(bill): What should the label's "scope" be?
check_label(ctx, fs->label, node);
if (fs->init != nullptr) {
check_stmt(ctx, fs->init, 0);
-5
View File
@@ -967,7 +967,6 @@ gb_internal void init_universal(void) {
add_global_bool_constant("true", true);
add_global_bool_constant("false", false);
// TODO(bill): Set through flags in the compiler
add_global_string_constant("ODIN_VENDOR", bc->ODIN_VENDOR);
add_global_string_constant("ODIN_VERSION", bc->ODIN_VERSION);
add_global_string_constant("ODIN_ROOT", bc->ODIN_ROOT);
@@ -1477,7 +1476,6 @@ gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMo
if (ctx->decl) {
mutex = &ctx->decl->type_and_value_mutex;
} else if (ctx->pkg) {
// TODO(bill): is a per package mutex is a good idea here?
mutex = &ctx->pkg->type_and_value_mutex;
}
@@ -2580,9 +2578,6 @@ gb_internal Array<EntityGraphNode *> generate_entity_dependency_graph(CheckerInf
}
}
// TODO(bill): This could be multithreaded to improve performance
// This means that the entity graph node set will have to be thread safe
TIME_SECTION("generate_entity_dependency_graph: Calculate edges for graph M - Part 2");
auto G = array_make<EntityGraphNode *>(allocator, 0, M.count);
-2
View File
@@ -387,8 +387,6 @@ struct CheckerInfo {
BlockingMutex foreign_mutex; // NOT recursive
StringMap<Entity *> foreigns;
// NOTE(bill): These are actually MPSC queues
// TODO(bill): Convert them to be MPSC queues
MPSCQueue<Entity *> definition_queue;
MPSCQueue<Entity *> entity_queue;
MPSCQueue<Entity *> required_global_variable_queue;
+3 -4
View File
@@ -287,7 +287,6 @@ gb_internal bool is_entity_kind_exported(EntityKind kind, bool allow_builtin = f
}
gb_internal bool is_entity_exported(Entity *e, bool allow_builtin = false) {
// TODO(bill): Determine the actual exportation rules for imports of entities
GB_ASSERT(e != nullptr);
if (!is_entity_kind_exported(e->kind, allow_builtin)) {
return false;
@@ -401,7 +400,7 @@ gb_internal Entity *alloc_entity_array_elem(Scope *scope, Token token, Type *typ
return entity;
}
gb_internal Entity *alloc_entity_procedure(Scope *scope, Token token, Type *signature_type, u64 tags) {
gb_internal Entity *alloc_entity_procedure(Scope *scope, Token token, Type *signature_type, u64 tags=0) {
Entity *entity = alloc_entity(Entity_Procedure, scope, token, signature_type);
entity->Procedure.tags = tags;
return entity;
@@ -418,7 +417,7 @@ gb_internal Entity *alloc_entity_import_name(Scope *scope, Token token, Type *ty
entity->ImportName.path = path;
entity->ImportName.name = name;
entity->ImportName.scope = import_scope;
entity->state = EntityState_Resolved; // TODO(bill): Is this correct?
entity->state = EntityState_Resolved;
return entity;
}
@@ -427,7 +426,7 @@ gb_internal Entity *alloc_entity_library_name(Scope *scope, Token token, Type *t
Entity *entity = alloc_entity(Entity_LibraryName, scope, token, type);
entity->LibraryName.paths = paths;
entity->LibraryName.name = name;
entity->state = EntityState_Resolved; // TODO(bill): Is this correct?
entity->state = EntityState_Resolved;
return entity;
}
+2 -5
View File
@@ -26,8 +26,8 @@ enum ExactValueKind {
ExactValue_Complex = 5,
ExactValue_Quaternion = 6,
ExactValue_Pointer = 7,
ExactValue_Compound = 8, // TODO(bill): Is this good enough?
ExactValue_Procedure = 9, // TODO(bill): Is this good enough?
ExactValue_Compound = 8,
ExactValue_Procedure = 9,
ExactValue_Typeid = 10,
ExactValue_Count,
@@ -101,7 +101,6 @@ gb_internal ExactValue exact_value_bool(bool b) {
}
gb_internal ExactValue exact_value_string(String string) {
// TODO(bill): Allow for numbers with underscores in them
ExactValue result = {ExactValue_String};
result.value_string = string;
return result;
@@ -702,7 +701,6 @@ gb_internal void match_exact_values(ExactValue *x, ExactValue *y) {
compiler_error("match_exact_values: How'd you get here? Invalid ExactValueKind %d", x->kind);
}
// TODO(bill): Allow for pointer arithmetic? Or are pointer slices good enough?
gb_internal ExactValue exact_binary_operator_value(TokenKind op, ExactValue x, ExactValue y) {
match_exact_values(&x, &y);
@@ -943,7 +941,6 @@ gb_internal bool compare_exact_values(TokenKind op, ExactValue x, ExactValue y)
case ExactValue_String: {
String a = x.value_string;
String b = y.value_string;
// TODO(bill): gb_memcompare is used because the strings are UTF-8
switch (op) {
case Token_CmpEq: return a == b;
case Token_NotEq: return a != b;
+1 -2
View File
@@ -1895,8 +1895,8 @@ gb_internal LLVMTypeRef lb_type_internal(lbModule *m, Type *type) {
case Type_SimdVector:
return lb_type_internal(m, base);
// TODO(bill): Deal with this correctly. Can this be named?
case Type_Proc:
// TODO(bill): Deal with this correctly. Can this be named?
return lb_type_internal(m, base);
case Type_Tuple:
@@ -2869,7 +2869,6 @@ gb_internal lbValue lb_find_value_from_entity(lbModule *m, Entity *e) {
if (USE_SEPARATE_MODULES) {
lbModule *other_module = lb_module_of_entity(m->gen, e);
// TODO(bill): correct this logic
bool is_external = other_module != m;
if (!is_external) {
if (e->code_gen_module != nullptr) {
+1 -4
View File
@@ -362,7 +362,6 @@ gb_internal lbProcedure *lb_create_dummy_procedure(lbModule *m, String link_name
Type *pt = p->type;
lbCallingConventionKind cc_kind = lbCallingConvention_C;
// TODO(bill): Clean up this logic
if (!is_arch_wasm()) {
cc_kind = lb_calling_convention_map[pt->Proc.calling_convention];
}
@@ -1702,7 +1701,6 @@ gb_internal lbValue lb_build_builtin_proc(lbProcedure *p, Ast *expr, TypeAndValu
lbValue v = lb_build_expr(p, ce->args[0]);
Type *t = base_type(v.type);
if (is_type_pointer(t)) {
// IMPORTANT TODO(bill): Should there be a nil pointer check?
v = lb_emit_load(p, v);
t = type_deref(t);
}
@@ -1730,7 +1728,6 @@ gb_internal lbValue lb_build_builtin_proc(lbProcedure *p, Ast *expr, TypeAndValu
lbValue v = lb_build_expr(p, ce->args[0]);
Type *t = base_type(v.type);
if (is_type_pointer(t)) {
// IMPORTANT TODO(bill): Should there be a nil pointer check?
v = lb_emit_load(p, v);
t = type_deref(t);
}
@@ -3144,7 +3141,7 @@ gb_internal lbValue lb_build_call_expr(lbProcedure *p, Ast *expr) {
lbValue res = lb_build_call_expr_internal(p, expr);
if (ce->optional_ok_one) { // TODO(bill): Minor hack for #optional_ok procedures
if (ce->optional_ok_one) {
GB_ASSERT(is_type_tuple(res.type));
GB_ASSERT(res.type->Tuple.variables.count == 2);
return lb_emit_struct_ev(p, res, 0);
+1 -4
View File
@@ -1688,7 +1688,6 @@ gb_internal void lb_build_type_switch_stmt(lbProcedure *p, AstTypeSwitchStmt *ss
lb_add_entity(p->module, case_entity, ptr);
lb_add_debug_local_variable(p, ptr.value, case_entity->type, case_entity->token);
} else {
// TODO(bill): is the correct expected behaviour?
lb_store_type_case_implicit(p, clause, parent_value);
}
@@ -2014,12 +2013,10 @@ gb_internal void lb_build_if_stmt(lbProcedure *p, Ast *node) {
defer (lb_close_scope(p, lbDeferExit_Default, nullptr));
if (is->init != nullptr) {
// TODO(bill): Should this have a separate block to begin with?
#if 1
lbBlock *init = lb_create_block(p, "if.init");
lb_emit_jump(p, init);
lb_start_block(p, init);
#endif
lb_build_stmt(p, is->init);
}
lbBlock *then = lb_create_block(p, "if.then");
-1
View File
@@ -731,7 +731,6 @@ gb_internal void lb_setup_type_info_data(lbProcedure *p) { // NOTE(bill): Setup
type_set_offsets(t); // NOTE(bill): Just incase the offsets have not been set yet
for (isize source_index = 0; source_index < count; source_index++) {
// TODO(bill): Order fields in source order not layout order
Entity *f = t->Struct.fields[source_index];
lbValue tip = lb_type_info(m, f->type);
i64 foffset = 0;
-2
View File
@@ -4968,7 +4968,6 @@ gb_internal bool init_parser(Parser *p) {
gb_internal void destroy_parser(Parser *p) {
GB_ASSERT(p != nullptr);
// TODO(bill): Fix memory leak
for (AstPackage *pkg : p->packages) {
for (AstFile *file : pkg->files) {
destroy_ast_file(file);
@@ -5012,7 +5011,6 @@ gb_internal WORKER_TASK_PROC(parser_worker_proc) {
gb_internal void parser_add_file_to_process(Parser *p, AstPackage *pkg, FileInfo fi, TokenPos pos) {
// TODO(bill): Use a better allocator
ImportedFile f = {pkg, fi, pos, p->file_to_process_count++};
auto wd = gb_alloc_item(permanent_allocator(), ParserWorkerData);
wd->parser = p;
+13 -9
View File
@@ -143,6 +143,7 @@ struct TypeStruct {
Type * soa_elem;
i32 soa_count;
StructSoaKind soa_kind;
BlockingMutex mutex; // for settings offsets
bool is_polymorphic;
bool are_offsets_set : 1;
@@ -244,6 +245,7 @@ struct TypeProc {
TYPE_KIND(Tuple, struct { \
Slice<Entity *> variables; /* Entity_Variable */ \
i64 * offsets; \
BlockingMutex mutex; /* for settings offsets */ \
bool are_offsets_being_processed; \
bool are_offsets_set; \
bool is_packed; \
@@ -821,6 +823,9 @@ gb_internal void type_path_pop(TypePath *tp) {
#define FAILURE_ALIGNMENT 0
gb_internal bool type_ptr_set_update(PtrSet<Type *> *s, Type *t) {
if (t == nullptr) {
return true;
}
if (ptr_set_exists(s, t)) {
return true;
}
@@ -829,13 +834,17 @@ gb_internal bool type_ptr_set_update(PtrSet<Type *> *s, Type *t) {
}
gb_internal bool type_ptr_set_exists(PtrSet<Type *> *s, Type *t) {
if (t == nullptr) {
return true;
}
if (ptr_set_exists(s, t)) {
return true;
}
// TODO(bill, 2019-10-05): This is very slow and it's probably a lot
// faster to cache types correctly
for (Type *f : *s) {
for (Type *f : *s) if (f->kind == t->kind) {
if (are_types_identical(t, f)) {
ptr_set_add(s, t);
return true;
@@ -2666,7 +2675,6 @@ gb_internal bool are_types_identical_internal(Type *x, Type *y, bool check_tuple
x->Struct.soa_kind == y->Struct.soa_kind &&
x->Struct.soa_count == y->Struct.soa_count &&
are_types_identical(x->Struct.soa_elem, y->Struct.soa_elem)) {
// TODO(bill); Fix the custom alignment rule
for_array(i, x->Struct.fields) {
Entity *xf = x->Struct.fields[i];
Entity *yf = y->Struct.fields[i];
@@ -2807,7 +2815,6 @@ gb_internal i64 union_tag_size(Type *u) {
return 0;
}
// TODO(bill): Is this an okay approach?
i64 max_align = 1;
if (u->Union.variants.count < 1ull<<8) {
@@ -2817,7 +2824,7 @@ gb_internal i64 union_tag_size(Type *u) {
} else if (u->Union.variants.count < 1ull<<32) {
max_align = 4;
} else {
GB_PANIC("how many variants do you have?!");
compiler_error("how many variants do you have?! %lld", cast(long long)u->Union.variants.count);
}
for_array(i, u->Union.variants) {
@@ -3136,8 +3143,6 @@ gb_internal Selection lookup_field_with_selection(Type *type_, String field_name
switch (type->Basic.kind) {
case Basic_any: {
#if 1
// IMPORTANT TODO(bill): Should these members be available to should I only allow them with
// `Raw_Any` type?
String data_str = str_lit("data");
String id_str = str_lit("id");
gb_local_persist Entity *entity__any_data = alloc_entity_field(nullptr, make_token_ident(data_str), t_rawptr, false, 0);
@@ -3663,10 +3668,9 @@ gb_internal i64 *type_set_offsets_of(Slice<Entity *> const &fields, bool is_pack
}
gb_internal bool type_set_offsets(Type *t) {
MUTEX_GUARD(&g_type_mutex); // TODO(bill): only per struct
t = base_type(t);
if (t->kind == Type_Struct) {
MUTEX_GUARD(&t->Struct.mutex);
if (!t->Struct.are_offsets_set) {
t->Struct.are_offsets_being_processed = true;
t->Struct.offsets = type_set_offsets_of(t->Struct.fields, t->Struct.is_packed, t->Struct.is_raw_union);
@@ -3675,6 +3679,7 @@ gb_internal bool type_set_offsets(Type *t) {
return true;
}
} else if (is_type_tuple(t)) {
MUTEX_GUARD(&t->Tuple.mutex);
if (!t->Tuple.are_offsets_set) {
t->Tuple.are_offsets_being_processed = true;
t->Tuple.offsets = type_set_offsets_of(t->Tuple.variables, t->Tuple.is_packed, false);
@@ -3849,7 +3854,6 @@ gb_internal i64 type_size_of_internal(Type *t, TypePath *path) {
max = size;
}
}
// TODO(bill): Is this how it should work?
return align_formula(max, align);
} else {
i64 count = 0, size = 0, align = 0;