Add is_dll_import to @extern, to support __declspec(dllimport) with the MSVC ABI

[kcbanner]

Closes #15903
Closes #21749

This change adds is_dll_import: bool to the options to @extern. This has the effect of setting the LLVM DLL Storage Class of the global to dllimport, which then generates the correct code to load from such a symbol at runtime - by prepending __imp_ to the symbol name and loading from that symbol instead.

This is necessary for the msvc ABI, as the "auto imports" features that make this work on -gnu are MinGW specific and don't exist there (see https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fauto-import).

As part of this change I also added an error note that explains why you can't place an @extern that uses either is_thread_local or is_dll_import in a comptime scope (because the address is runtime-known). To do this I added a new optional field to NeededComptimeReason which does seem a bit heavy-handed since it's only used in this one place - I can easily change this to build one single, longer needed_comptime_reason if that is preferred.

Other changes:

• tests/standalone/extern wasn't actually running the test step, that is fixed now

A question for reviewers about existing behaviour:

No compile errors are currently emitted when using optional pointers in @extern in comptime scopes - as an example:

const foo_opt_data = @extern(?*u32, .{
    .name = "foo_opt_data",
    .linkage = .strong,
    .is_dll_import = true,
});

This doesn't result in a compile error, but a linker error instead, because a @main.foo_opt_data global is generated that is initialized from a non-existent symbol @foo_opt_data, this can't be available at comptime because the value is only known at runtime.

@main.foo_opt_data = internal unnamed_addr constant ptr @foo_opt_data, align 8, !dbg !3934

I tried to determine the cause for this, and I thought it was isPtrRuntimeValue not unwrapping the optional properly when getting the backing nav, but it seems to handle that in ip.getBackinNav. I'm happy to fix this as part of this pr and add a compile error test case for this to builtin_extern_in_comptime_scope, if someone can point me in the right direction.

[andrewrk]

Good work, I was going to press merge but then I saw this question:

A question for reviewers about existing behaviour:

what does this have to do with optionals? seems to have to only do with comptime?

[kcbanner]

What I meant there is that I expected to able to apply this diff and have this test pass:

--- a/test/cases/compile_errors/builtin_extern_in_comptime_scope.zig
+++ b/test/cases/compile_errors/builtin_extern_in_comptime_scope.zig
@@ -1,5 +1,7 @@
 const foo_tl = @extern(*i32, .{ .name = "foo", .is_thread_local = true });
+const opt_tl = @extern(?*i32, .{ .name = "foo", .is_thread_local = true });
 const foo_dll = @extern(*i32, .{ .name = "foo", .is_dll_import = true });
+const opt_dll = @extern(?*i32, .{ .name = "foo", .is_dll_import = true });
 pub export fn entry() void {
     _ = foo_tl;
 }
@@ -13,6 +15,12 @@ pub export fn entry2() void {
 // :1:16: error: unable to resolve comptime value
 // :1:16: note: global variable initializer must be comptime-known
 // :1:16: note: thread local and dll imported variables have runtime-known addresses
+// :2:16: error: unable to resolve comptime value
+// :2:16: note: global variable initializer must be comptime-known
+// :2:16: note: thread local and dll imported variables have runtime-known addresses
 // :3:17: error: unable to resolve comptime value
 // :3:17: note: global variable initializer must be comptime-known
 // :3:17: note: thread local and dll imported variables have runtime-known addresses
+// :4:17: error: unable to resolve comptime value
+// :4:17: note: global variable initializer must be comptime-known
+// :4:17: note: thread local and dll imported variables have runtime-known addresses

But in reality there are no compile errors for the ?*u32 versions - which confused me since since it looked like the logic in isPtrRuntimeValue was correct in unwrapping the optional type on it's way to retrieve the backing Nav to see if this is indeed is_dll_import or is_thread_local.

I'm happy to leave that enhancement to a future PR though - assuming my assumption about those being compile errors is correct.

[andrewrk]

I agree with you, that test should pass. However I see no need for it to block this nice patch. Thanks!