Implement @externWeak builtin

[LemonBoy]

This PR implements the proposal outlined in #1917

[andrewrk]

Messing with result locations is not needed to implement this feature:

diff --git a/src/all_types.hpp b/src/all_types.hpp
index f8cd132b4..f7bd7ad45 100644
--- a/src/all_types.hpp
+++ b/src/all_types.hpp
@@ -4065,14 +4065,12 @@ struct IrInstructionExternWeakSrc {
 
     IrInstruction *name;
     IrInstruction *ptr_type;
-    ResultLoc *result_loc;
 };
 
 struct IrInstructionExternWeakGen {
     IrInstruction base;
 
     Buf *name;
-    IrInstruction *result_loc;
 };
 
 enum ResultLocId {
diff --git a/src/codegen.cpp b/src/codegen.cpp
index 537092e13..bb089f45b 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -6218,28 +6218,7 @@ static LLVMValueRef ir_render_extern_weak(CodeGen *g, IrExecutable *executable,
         zig_unreachable();
     }
 
-    LLVMValueRef sym_ptr = LLVMBuildBitCast(g->builder, global_sym, get_llvm_type(g, ptr_type), "");
-
-    LLVMValueRef result_loc = instruction->result_loc ? ir_llvm_value(g, instruction->result_loc) : nullptr;
-
-    if (!handle_is_ptr(wanted_type)) {
-        if (result_loc != nullptr) {
-            gen_store_untyped(g, sym_ptr, result_loc, 0, false);
-        }
-        return sym_ptr;
-    }
-
-    if (result_loc != nullptr) {
-        LLVMValueRef usize_zero = LLVMConstNull(g->builtin_types.entry_usize->llvm_type);
-        LLVMValueRef nonnull_bit = LLVMBuildICmp(g->builder, LLVMIntNE, sym_ptr, usize_zero, "");
-
-        LLVMValueRef val_ptr = LLVMBuildStructGEP(g->builder, result_loc, maybe_child_index, "");
-        gen_assign_raw(g, val_ptr, ptr_type, sym_ptr);
-        LLVMValueRef maybe_ptr = LLVMBuildStructGEP(g->builder, result_loc, maybe_null_index, "");
-        gen_store_untyped(g, nonnull_bit, maybe_ptr, 0, false);
-    }
-
-    return result_loc;
+    return LLVMBuildBitCast(g->builder, global_sym, get_llvm_type(g, ptr_type), "");
 }
 
 static void set_debug_location(CodeGen *g, IrInstruction *instruction) {
diff --git a/src/ir.cpp b/src/ir.cpp
index 001b40a91..8e7ff9229 100644
--- a/src/ir.cpp
+++ b/src/ir.cpp
@@ -1614,13 +1614,12 @@ static T *ir_build_instruction(IrBuilder *irb, Scope *scope, AstNode *source_nod
     return special_instruction;
 }
 
-static IrInstruction *ir_build_extern_weak_src(IrBuilder *irb, Scope *scope, AstNode *source_node, IrInstruction *name,
-    IrInstruction *ptr_type, ResultLoc *result_loc)
+static IrInstruction *ir_build_extern_weak_src(IrBuilder *irb, Scope *scope, AstNode *source_node,
+        IrInstruction *name, IrInstruction *ptr_type)
 {
     IrInstructionExternWeakSrc *instruction = ir_build_instruction<IrInstructionExternWeakSrc>(irb, scope, source_node);
     instruction->name = name;
     instruction->ptr_type = ptr_type;
-    instruction->result_loc = result_loc;
 
     ir_ref_instruction(name, irb->current_basic_block);
     ir_ref_instruction(ptr_type, irb->current_basic_block);
@@ -1628,14 +1627,9 @@ static IrInstruction *ir_build_extern_weak_src(IrBuilder *irb, Scope *scope, Ast
     return &instruction->base;
 }
 
-static IrInstruction *ir_build_extern_weak_gen(IrBuilder *irb, Scope *scope, AstNode *source_node, Buf *name,
-    IrInstruction *result_loc)
-{
+static IrInstruction *ir_build_extern_weak_gen(IrBuilder *irb, Scope *scope, AstNode *source_node, Buf *name) {
     IrInstructionExternWeakGen *instruction = ir_build_instruction<IrInstructionExternWeakGen>(irb, scope, source_node);
     instruction->name = name;
-    instruction->result_loc = result_loc;
-
-    ir_ref_instruction(result_loc, irb->current_basic_block);
 
     return &instruction->base;
 }
@@ -6505,7 +6499,7 @@ static IrInstruction *ir_gen_builtin_fn_call(IrBuilder *irb, Scope *scope, AstNo
                 if (arg1_value == irb->codegen->invalid_instruction)
                     return arg1_value;
 
-                IrInstruction *extern_weak = ir_build_extern_weak_src(irb, scope, node, arg0_value, arg1_value, result_loc);
+                IrInstruction *extern_weak = ir_build_extern_weak_src(irb, scope, node, arg0_value, arg1_value);
                 return ir_lval_wrap(irb, scope, extern_weak, lval, result_loc);
             }
         case BuiltinFnIdUnionInit:
@@ -26820,14 +26814,8 @@ static IrInstruction *ir_analyze_instruction_extern_weak_src(IrAnalyze *ira, IrI
 
     ZigType *opt_ptr_type = get_optional_type(ira->codegen, ptr_type);
 
-    IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc,
-            opt_ptr_type, nullptr, true, false, true);
-    if (type_is_invalid(result_loc->value->type) || instr_is_unreachable(result_loc)) {
-        return result_loc;
-    }
-
     IrInstruction *result = ir_build_extern_weak_gen(&ira->new_irb, instruction->base.scope,
-        instruction->base.source_node, symbol_name, result_loc);
+        instruction->base.source_node, symbol_name);
     result->value->type = opt_ptr_type;
 
     return result;
diff --git a/src/ir_print.cpp b/src/ir_print.cpp
index b81473618..c2f44e01a 100644
--- a/src/ir_print.cpp
+++ b/src/ir_print.cpp
@@ -2102,15 +2102,11 @@ static void ir_print_extern_weak_src(IrPrint *irp, IrInstructionExternWeakSrc *i
     ir_print_other_instruction(irp, instruction->name);
     fprintf(irp->f, ",");
     ir_print_other_instruction(irp, instruction->ptr_type);
-    fprintf(irp->f, ",");
-    ir_print_result_loc(irp, instruction->result_loc);
     fprintf(irp->f, ")");
 }
 
 static void ir_print_extern_weak_gen(IrPrint *irp, IrInstructionExternWeakGen *instruction) {
-    fprintf(irp->f, "@externWeak(\"%s\",", buf_ptr(instruction->name));
-    ir_print_other_instruction(irp, instruction->result_loc);
-    fprintf(irp->f, ")");
+    fprintf(irp->f, "@externWeak(\"%s\")", buf_ptr(instruction->name));
 }
 
 static void ir_print_spill_begin(IrPrint *irp, IrInstructionSpillBegin *instruction) {

Here are some docs to put into doc/langref.html.in to help clarify how the type parameter is supposed to work:

      {#header_open|@externWeak#}
      <pre>{#syntax#}@externWeak(comptime symbol_name: []const u8, SymbolType: type) ?*T{#endsyntax#}</pre>
      <p>
      Returns a pointer to an external symbol, or `null` if no such symbol
      is linked in.
      </p>
      <p>
      Normally, external symbols are <strong>strong</strong>, which means an application
      receives a linker error if the symbol is not available at link time.
      <strong>Weak</strong> symbols are optional. This builtin allows code
      to detect at runtime whether such symbols were provided at link time.
      </p>
      {#header_close#}

The type parameter is not necessarily a pointer, it's the type of the symbol, and then @externWeak will give you an optional pointer to that type. Make sense?

[LemonBoy]

Messing with result locations is not needed to implement this feature:

If you don't take pointer types (thus making it impossible to ask for an allowzero pointer) then the result location can be avoided.

The type parameter is not necessarily a pointer, it's the type of the symbol, and then @externWeak will give you an optional pointer to that type.

No strong preference on my end, if you feel it's better to just ask for a type I'll change the implementation accordingly.

[LemonBoy]

No strong preference on my end, if you feel it's better to just ask for a type I'll change the implementation accordingly.

Oh, here's a possible advantage: you can easily ask for a const pointer without having to go trough a ptrCast. It's not much of a problem but that use case came to my mind while adjusting the patch.

[andrewrk]

@LemonBoy do you want to hand this one off to me? Otherwise I'm still open to reconsidering if you wanted to press the case for the other API you proposed.

[LemonBoy]

@LemonBoy do you want to hand this one off to me? Otherwise I'm still open to reconsidering if you wanted to press the case for the other API you proposed.

I'm still torn about this, I really don't like how a builtin call (maybe hidden behind various layers of abstractions) has the same side effects of a global symbol declaration.

Wrt the API, I found this quote in the original proposal:

PointerType could be any pointer type. The reason we supply the pointer type and not the element type is so that the default alignment can optionally be overridden by using a pointer type with an explicit alignment.
So it's something more to think about.

[andrewrk]

I'm still torn about this, I really don't like how a builtin call (maybe hidden behind various layers of abstractions) has the same side effects of a global symbol declaration.

Wrt the API, I found this quote in the original proposal:

Makes sense. Note that we have precedent for this with @export though. And it's turned out nicely in the start.zig file:

zig/lib/std/start.zig

Lines 22 to 44 in b5ac079

	comptime {
	if (builtin.output_mode == .Lib and builtin.link_mode == .Dynamic) {
	if (builtin.os == .windows and !@hasDecl(root, "_DllMainCRTStartup")) {
	@export(_DllMainCRTStartup, .{ .name = "_DllMainCRTStartup" });
	}
	} else if (builtin.output_mode == .Exe or @hasDecl(root, "main")) {
	if (builtin.link_libc and @hasDecl(root, "main")) {
	if (@typeInfo(@TypeOf(root.main)).Fn.calling_convention != .C) {
	@export(main, .{ .name = "main", .linkage = .Weak });
	}
	} else if (builtin.os == .windows) {
	if (!@hasDecl(root, "WinMain") and !@hasDecl(root, "WinMainCRTStartup")) {
	@export(WinMainCRTStartup, .{ .name = "WinMainCRTStartup" });
	}
	} else if (builtin.os == .uefi) {
	if (!@hasDecl(root, "EfiMain")) @export(EfiMain, .{ .name = "EfiMain" });
	} else if (is_wasm and builtin.os == .freestanding) {
	if (!@hasDecl(root, start_sym_name)) @export(wasm_freestanding_start, .{ .name = start_sym_name });
	} else if (builtin.os != .other and builtin.os != .freestanding) {
	if (!@hasDecl(root, start_sym_name)) @export(_start, .{ .name = start_sym_name });
	}
	}
	}

Hmm. What about making it even more generic - @extern rather than @externWeak - and taking a struct of options, with defaults?

pub const ExternOptions = struct {
    /// If this is `GlobalLinkage.Weak`, `@extern` returns an optional pointer. Otherwise,
    /// it returns a non-optional pointer.
    linkage: GlobalLinkage = .Strong,
    is_const: bool = false,
    is_thread_local: bool = false,
    /// `null` means to use ABI alignment. Otherwise must be a non-zero power of two.
    alignment: ?u29 = null,
};

Yeah, thread local externs are a thing:

zig/lib/std/c/dragonfly.zig

Line 3 in b5ac079

extern "c" threadlocal var errno: c_int;

I think I'm in favor of this approach.

@extern(comptime name: []const u8, comptime T: type, comptime options: std.builtin.ExternOptions) var

I would also be OK with putting name and T in the options struct and having @extern take a single parameter.

[andrewrk]

I added this comment so that this diff is not lost to the world, and I'm closing the PR since it's not merge-ready and hasn't seen any progress in over a month.