Resolve other TODOs

2 files changed, 91 insertions, 89 deletions

diff --git a/src/check_decl.cpp b/src/check_decl.cpp
index 2be5be023..21c57e977 100644
--- a/src/check_decl.cpp
+++ b/src/check_decl.cpp
@@ -551,7 +551,7 @@ gb_internal void check_type_decl(CheckerContext *ctx, Entity *e, Ast *init_expr,
 					mpsc_enqueue(&ctx->checker->procs_with_objc_context_provider_to_check, e);
 				}
 
-				// @TODO(harold): I think there's a Check elsewhere in the checker for checking cycles.
+				// TODO(harold): I think there's a Check elsewhere in the checker for checking cycles.
 				//					See about moving this to the right location.
 				// Ensure superclass hierarchy are all Objective-C classes and does not cycle
 
@@ -1013,108 +1013,107 @@ gb_internal void check_objc_methods(CheckerContext *ctx, Entity *e, AttributeCon
 	} else {
 		Type *t = ac.objc_type;
 
-		if (t->kind == Type_Named) {	// TODO(harold): Shouldn't this be an error otherwise? Or is it checked elsehwere?
-			Entity *tn = t->Named.type_name;
+		GB_ASSERT(t->kind == Type_Named);	// NOTE(harold): This is already checked for at the attribute resolution stage.
+		Entity *tn = t->Named.type_name;
 
-			GB_ASSERT(tn->kind == Entity_TypeName);
+		GB_ASSERT(tn->kind == Entity_TypeName);
 
-			if (tn->scope != e->scope) {
-				error(e->token, "@(objc_name) attribute may only be applied to procedures and types within the same scope");
-			} else {
+		if (tn->scope != e->scope) {
+			error(e->token, "@(objc_name) attribute may only be applied to procedures and types within the same scope");
+		} else {
 
-				// Enable implementation by default if the class is an implementer too and
-				// @objc_implement was not set to false explicitly in this proc.
-				bool implement = tn->TypeName.objc_is_implementation;
-				if (ac.objc_is_disabled_implement) {
-					implement = false;
-				}
+			// Enable implementation by default if the class is an implementer too and
+			// @objc_implement was not set to false explicitly in this proc.
+			bool implement = tn->TypeName.objc_is_implementation;
+			if (ac.objc_is_disabled_implement) {
+				implement = false;
+			}
 
-				if (implement) {
-					GB_ASSERT(e->kind == Entity_Procedure);
-
-					auto &proc = e->type->Proc;
-					Type *first_param = proc.param_count > 0 ? proc.params->Tuple.variables[0]->type : t_untyped_nil;
-
-					if (!tn->TypeName.objc_is_implementation) {
-						error(e->token, "@(objc_is_implement) attribute may only be applied to procedures whose class also have @(objc_is_implement) applied");
-					} else if (!ac.objc_is_class_method && !(first_param->kind == Type_Pointer && internal_check_is_assignable_to(t, first_param->Pointer.elem))) {
-						error(e->token, "Objective-C instance methods implementations require the first parameter to be a pointer to the class type set by @(objc_type)");
-					} else if (proc.calling_convention == ProcCC_Odin && !tn->TypeName.objc_context_provider) {
-						error(e->token, "Objective-C methods with Odin calling convention can only be used with classes that have @(objc_context_provider) set");
-					} else if (ac.objc_is_class_method && proc.calling_convention != ProcCC_CDecl) {
-						error(e->token, "Objective-C class methods (objc_is_class_method=true) that have @objc_is_implementation can only use \"c\" calling convention");
-					} else if (proc.result_count > 1) {
-						error(e->token, "Objective-C method implementations may return at most 1 value");
-					} else {
-						// Always export unconditionally
-						// NOTE(harold): This means check_objc_methods() MUST be called before
-						//				 e->Procedure.is_export is set in check_proc_decl()!
-						if (ac.is_export) {
-							error(e->token, "Explicit export not allowed when @(objc_implement) is set. It set exported implicitly");
-						}
-						if (ac.link_name != "") {
-							error(e->token, "Explicit linkage not allowed when @(objc_implement) is set. It set to \"strong\" implicitly");
-						}
+			if (implement) {
+				GB_ASSERT(e->kind == Entity_Procedure);
+
+				auto &proc = e->type->Proc;
+				Type *first_param = proc.param_count > 0 ? proc.params->Tuple.variables[0]->type : t_untyped_nil;
+
+				if (!tn->TypeName.objc_is_implementation) {
+					error(e->token, "@(objc_is_implement) attribute may only be applied to procedures whose class also have @(objc_is_implement) applied");
+				} else if (!ac.objc_is_class_method && !(first_param->kind == Type_Pointer && internal_check_is_assignable_to(t, first_param->Pointer.elem))) {
+					error(e->token, "Objective-C instance methods implementations require the first parameter to be a pointer to the class type set by @(objc_type)");
+				} else if (proc.calling_convention == ProcCC_Odin && !tn->TypeName.objc_context_provider) {
+					error(e->token, "Objective-C methods with Odin calling convention can only be used with classes that have @(objc_context_provider) set");
+				} else if (ac.objc_is_class_method && proc.calling_convention != ProcCC_CDecl) {
+					error(e->token, "Objective-C class methods (objc_is_class_method=true) that have @objc_is_implementation can only use \"c\" calling convention");
+				} else if (proc.result_count > 1) {
+					error(e->token, "Objective-C method implementations may return at most 1 value");
+				} else {
+					// Always export unconditionally
+					// NOTE(harold): This means check_objc_methods() MUST be called before
+					//				 e->Procedure.is_export is set in check_proc_decl()!
+					if (ac.is_export) {
+						error(e->token, "Explicit export not allowed when @(objc_implement) is set. It set exported implicitly");
+					}
+					if (ac.link_name != "") {
+						error(e->token, "Explicit linkage not allowed when @(objc_implement) is set. It set to \"strong\" implicitly");
+					}
 
-						ac.is_export = true;
-						ac.linkage   = STR_LIT("strong");
+					ac.is_export = true;
+					ac.linkage   = STR_LIT("strong");
 
-						auto method = ObjcMethodData{ ac, e };
-						method.ac.objc_selector = ac.objc_selector != "" ? ac.objc_selector : ac.objc_name;
+					auto method = ObjcMethodData{ ac, e };
+					method.ac.objc_selector = ac.objc_selector != "" ? ac.objc_selector : ac.objc_name;
 
-						CheckerInfo *info = ctx->info;
-						mutex_lock(&info->objc_method_mutex);
-						defer (mutex_unlock(&info->objc_method_mutex));
+					CheckerInfo *info = ctx->info;
+					mutex_lock(&info->objc_method_mutex);
+					defer (mutex_unlock(&info->objc_method_mutex));
 
-						Array<ObjcMethodData>* method_list = map_get(&info->objc_method_implementations, t);
-						if (method_list) {
-							array_add(method_list, method);
-						} else {
-							auto list = array_make<ObjcMethodData>(permanent_allocator(), 1, 8);
-							list[0] = method;
+					Array<ObjcMethodData>* method_list = map_get(&info->objc_method_implementations, t);
+					if (method_list) {
+						array_add(method_list, method);
+					} else {
+						auto list = array_make<ObjcMethodData>(permanent_allocator(), 1, 8);
+						list[0] = method;
 
-							map_set(&info->objc_method_implementations, t, list);
-						}
+						map_set(&info->objc_method_implementations, t, list);
 					}
-				} else if (ac.objc_selector != "") {
-					error(e->token, "@(objc_selector) may only be applied to procedures that are Objective-C implementations.");
 				}
+			} else if (ac.objc_selector != "") {
+				error(e->token, "@(objc_selector) may only be applied to procedures that are Objective-C implementations.");
+			}
 
-				mutex_lock(&global_type_name_objc_metadata_mutex);
-				defer (mutex_unlock(&global_type_name_objc_metadata_mutex));
+			mutex_lock(&global_type_name_objc_metadata_mutex);
+			defer (mutex_unlock(&global_type_name_objc_metadata_mutex));
 
-				if (!tn->TypeName.objc_metadata) {
-					tn->TypeName.objc_metadata = create_type_name_obj_c_metadata();
-				}
-				auto *md = tn->TypeName.objc_metadata;
-				mutex_lock(md->mutex);
-				defer (mutex_unlock(md->mutex));
-
-				if (!ac.objc_is_class_method) {
-					bool ok = true;
-					for (TypeNameObjCMetadataEntry const &entry : md->value_entries) {
-						if (entry.name == ac.objc_name) {
-							error(e->token, "Previous declaration of @(objc_name=\"%.*s\")", LIT(ac.objc_name));
-							ok = false;
-							break;
-						}
-					}
-					if (ok) {
-						array_add(&md->value_entries, TypeNameObjCMetadataEntry{ac.objc_name, e});
-					}
-				} else {
-					bool ok = true;
-					for (TypeNameObjCMetadataEntry const &entry : md->type_entries) {
-						if (entry.name == ac.objc_name) {
-							error(e->token, "Previous declaration of @(objc_name=\"%.*s\")", LIT(ac.objc_name));
-							ok = false;
-							break;
-						}
+			if (!tn->TypeName.objc_metadata) {
+				tn->TypeName.objc_metadata = create_type_name_obj_c_metadata();
+			}
+			auto *md = tn->TypeName.objc_metadata;
+			mutex_lock(md->mutex);
+			defer (mutex_unlock(md->mutex));
+
+			if (!ac.objc_is_class_method) {
+				bool ok = true;
+				for (TypeNameObjCMetadataEntry const &entry : md->value_entries) {
+					if (entry.name == ac.objc_name) {
+						error(e->token, "Previous declaration of @(objc_name=\"%.*s\")", LIT(ac.objc_name));
+						ok = false;
+						break;
 					}
-					if (ok) {
-						array_add(&md->type_entries, TypeNameObjCMetadataEntry{ac.objc_name, e});
+				}
+				if (ok) {
+					array_add(&md->value_entries, TypeNameObjCMetadataEntry{ac.objc_name, e});
+				}
+			} else {
+				bool ok = true;
+				for (TypeNameObjCMetadataEntry const &entry : md->type_entries) {
+					if (entry.name == ac.objc_name) {
+						error(e->token, "Previous declaration of @(objc_name=\"%.*s\")", LIT(ac.objc_name));
+						ok = false;
+						break;
 					}
 				}
+				if (ok) {
+					array_add(&md->type_entries, TypeNameObjCMetadataEntry{ac.objc_name, e});
+				}
 			}
 		}
 	}
diff --git a/src/llvm_backend.cpp b/src/llvm_backend.cpp
index ce40e6e9f..10aa45b14 100644
--- a/src/llvm_backend.cpp
+++ b/src/llvm_backend.cpp
@@ -1173,7 +1173,6 @@ gb_internal lbProcedure *lb_create_objc_names(lbModule *main_module) {
 	return p;
 }
 
-// TODO(harold): Perhaps move this out of here and into a more suitable place?
 String lb_get_objc_type_encoding(Type *t, isize pointer_depth = 0) {
 	// NOTE(harold): See https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html#//apple_ref/doc/uid/TP40008048-CH100
 
@@ -1424,6 +1423,7 @@ String lb_get_objc_type_encoding(Type *t, isize pointer_depth = 0) {
 	#undef INT_SIZE_ENCODING
 
 	GB_PANIC("Unreachable");
+	return str_lit("");
 }
 
 struct lbObjCGlobalClass {
@@ -1778,7 +1778,10 @@ gb_internal void lb_finalize_objc_names(lbGenerator *gen, lbProcedure *p) {
 			const i64 size      = type_size_of(ivar_base);
 			const i64 alignment = (i64)floor_log2((u64)type_align_of(ivar_base));
 
-			// TODO(harold): Should we pass the actual type encoding? Might not be ideal for obfuscation.
+			// NOTE(harold): I've opted to not emit the type encoding for ivars in order to keep the data private.
+			//               If there is desire in the future to emit the type encoding for introspection through the Obj-C runtime,
+			//               then perhaps an option can be added for it then.
+			// Should we pass the actual type encoding? Might not be ideal for obfuscation.
 			String ivar_name  = str_lit("__$ivar");
 			String ivar_types = str_lit("{= }");	//lb_get_objc_type_encoding(ivar_type);
 			args.count = 5;