Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zig 0.10.0 #1491

Closed
wants to merge 51 commits into from
Closed

Zig 0.10.0 #1491

wants to merge 51 commits into from

Conversation

vjpr
Copy link
Contributor

@vjpr vjpr commented Nov 11, 2022

See: #1490

I'm trying to compile a list of things that need changing based on errors I see.

Build command

ZIG_FLAGS="-fstage1 -freference-trace" \
ZIG="/usr/local/Cellar/zig/0.10.0/bin/zig" \
CFLAGS="-Wno-int-conversion" \
make \
vendor \
identifier-cache \
bindings \
jsc \
dev

Fixes

error: invalid builtin function: '@minimum'

Fix: Rename @maximum and @minimum to @max and @min

error: no field named 'iterate' in struct 'fs.Dir.OpenDirOptions'

http://ratfactor.com/zig/stdlib-browseable/fs.zig.html

error: empty test name must be omitted

test "" -> test

error: expected 1 argument, found 2 - @popCount

/Users/Vaughan/dev/fork/+bun/bun/src/string_immutable.zig:586:21: error: expected 1 argument, found 2
        const cmp = @popCount(std.meta.Int(.unsigned, ascii_vector_size), @bitCast(@Vector(ascii_vector_size, u1), vec == splatted));
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: number '0000010' has leading zero

/Users/Vaughan/dev/fork/+bun/bun/src/install/lockfile.zig:1447:9: error: number '0000010' has leading zero
        0000010 | 0000100 | 0000001 | 0001000 | 0000040 | 0000004 | 0000002 | 0000400 | 0000200 | 0000020,
        ^~~~~~~
/Users/Vaughan/dev/fork/+bun/bun/src/install/lockfile.zig:1447:9: note: use '0o' prefix for octal literals

capture shadows declaration of 'str'

/Users/Vaughan/dev/fork/+bun/bun/src/install/npm.zig:1048:43: error: capture shadows declaration of 'str'
                            .e_string => |str| {
                                          ^~~
/Users/Vaughan/dev/fork/+bun/bun/src/install/npm.zig:617:9: note: declared here
    pub fn str(self: *const PackageManifest, external: ExternalString) string {
    ~~~~^~

pointless discard of function parameter

/Users/Vaughan/dev/fork/+bun/bun/src/http_client_async.zig:515:9: error: pointless discard of function parameter
    _ = socket;
        ^~~~~~
/Users/Vaughan/dev/fork/+bun/bun/src/http_client_async.zig:527:121: note: used here
                    client.done(comptime is_ssl, if (is_ssl) &http_thread.https_context else &http_thread.http_context, socket);
                                                                                                                        ^~~~~~

@ctz - expected 1 argument, found 2

/Users/Vaughan/dev/fork/+bun/bun/src/lock.zig:27:38: error: expected 1 argument, found 2
            return self.state.bitSet(@ctz(u32, LOCKED), .Acquire) == UNLOCKED;
                                     ^~~~~~~~~~~~~~~~~

Following ziglang's approach from here:

https://github.com/ziglang/zig/blob/df7223c7f2504b8f98526a86630bd6a7c07720a9/lib/std/Thread/Mutex.zig#L142-L145

HELP WANTED: error: declaration 'Type' shadows function parameter from outer scope

/Users/Vaughan/dev/fork/+bun/bun/src/ref_count.zig:78:19: error: declaration 'Type' shadows function parameter from outer scope
        pub const Type = Type;
                  ^~~~
/Users/Vaughan/dev/fork/+bun/bun/src/ref_count.zig:3:26: note: previous declaration here
pub fn RefCount(comptime Type: type, comptime deinit_on_zero: bool) type {
                         ^~~~

I used MyType to avoid shadowing...maybe you want something different.

error: pointless discard of function parameter

There are a bunch of these...

Not sure why they exist in the first place...maybe unreachable?

See: ziglang/zig#12803

There is a proposal to add unused = foo: ziglang/zig#10245

/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/event_loop.zig:782:13: error: pointless discard of function parameter
        _ = loop;
            ^~~~

function type cannot have a name

/Users/Vaughan/dev/fork/+bun/bun/src/panic_handler.zig:19:48: error: function type cannot have a name
pub fn NewPanicHandler(comptime panic_func: fn handle_panic(msg: []const u8, error_return_type: ?*std.builtin.StackTrace) noreturn) type {

pub const TypeInfo = @compileError("deprecated; use Type");

fmt - {s} -> {any}

enum 'src.tagged_pointer.enum:67:37' has no field named 'Microtask'

./src/bun.js/event_loop.zig:242:17: error: enum 'src.tagged_pointer.enum:69:37' has no field named 'Microtask'
                .Microtask => {
                ^
./src/tagged_pointer.zig:69:37: note: 'src.tagged_pointer.enum:69:37' declared here
            break :tag_break @Type(.{

...and FileBlobLoader, etc.

Related:

error: unable to evaluate constant expression

This is caused by using {} or {any} with comptimePrint. It tries to dereference a point which is not valid during comptime.

This was probably caused by me changing all {} to {any}.

/usr/local/Cellar/zig/0.10.0/lib/zig/std/fmt.zig:413:31: error: unable to evaluate constant expression
                try formatInt(@ptrToInt(value), 16, .lower, FormatOptions{}, writer);
                              ^
/usr/local/Cellar/zig/0.10.0/lib/zig/std/fmt.zig:477:29: note: called from here
        return formatAddress(value, options, writer);
                            ^
/usr/local/Cellar/zig/0.10.0/lib/zig/std/fmt.zig:183:23: note: called from here
        try formatType(
                      ^
/usr/local/Cellar/zig/0.10.0/lib/zig/std/fmt.zig:1934:11: note: called from here
    format(counting_writer.writer(), fmt, args) catch |err| switch (err) {};
          ^
/usr/local/Cellar/zig/0.10.0/lib/zig/std/fmt.zig:1978:76: note: called from here
pub fn comptimePrint(comptime fmt: []const u8, args: anytype) *const [count(fmt, args):0]u8 {
                                                                           ^
/usr/local/Cellar/zig/0.10.0/lib/zig/std/fmt.zig:1978:63: note: called from here
pub fn comptimePrint(comptime fmt: []const u8, args: anytype) *const [count(fmt, args):0]u8 {
                                                              ^
./src/http.zig:4056:42: note: called from here
  comptime RequestContext.printStatusLine(101);

sys/cdefs.h' file not found

./src/darwin_c.zig:3:21: error: C import failed
const sysResource = @cImport(@cInclude("sys/resource.h"));
                    ^
/usr/local/Cellar/zig/0.10.0/lib/zig/libc/include/any-macos-any/sys/resource.h:68:10: note: 'sys/cdefs.h' file not found
#include <sys/cdefs.h>

Zig using x86 build instead of arm64. Make sure the run eval opt/homebew.... command.

return val posix_spawnattr_destroy

pub fn deinit(self: *Attr) void {
        if (comptime bun.Environment.isMac) {
        // https://github.com/ziglang/zig/issues/12964
        system.posix_spawnattr_destroy(&self.attr);
        } else {
        _ = system.posix_spawnattr_destroy(&self.attr);
        }

        self.* = undefined;
}

ziglang/zig#12964

@alignCast

...

@Jarred-Sumner
Copy link
Collaborator

Thank you for this

Probably most tedious will be the openDirIterable changes

@Jarred-Sumner
Copy link
Collaborator

Jarred-Sumner commented Nov 11, 2022

The other thing we will need to do is update to llvm 15 everywhere. For that, we mostly use llvm's download script so it will be automatic for Docker

We will also need to rebuild WebKit with LLVM 15. I can do that part

@Jarred-Sumner
Copy link
Collaborator

sorry for the conflicts

@vjpr
Copy link
Contributor Author

vjpr commented Nov 14, 2022

Got zig build headers-obj -fstage1 building without errors 🚀


WebGLAny.h not found

/opt/homebrew/opt/llvm/bin/clang++ -I/Users/Vaughan/dev/fork/+bun/bun/src/deps/uws/uSockets/src -I/Users/Vaughan/dev/fork/+bun/bun/src/deps/uws/src -I/Users/Vaughan/dev/fork/+bun/bun/src/deps -I/Users/Vaughan/dev/fork/+bun/bun/src/deps/mimalloc/include -Isrc/napi -I/Users/Vaughan/dev/fork/+bun/bun/src/deps/boringssl/include -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/WTF/Headers -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/ICU/Headers -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/bmalloc/Headers -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/ -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/include -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/JavaScriptCore/PrivateHeaders -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/bmalloc/PrivateHeaders -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/WebKitBuild/Release/WTF/PrivateHeaders -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/ -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/builtins -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/webcore -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/webcrypto -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/sqlite -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/builtins/cpp -I -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/modules -I/Users/Vaughan/webkit-build/include -I/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/WebKit/Source  -std=c++2a -DSTATICALLY_LINKED_WITH_JavaScriptCore=1 -DSTATICALLY_LINKED_WITH_WTF=1 -DSTATICALLY_LINKED_WITH_BMALLOC=1 -DBUILDING_WITH_CMAKE=1 -DBUN_SINGLE_THREADED_PER_VM_ENTRY_SCOPE=1 -DNDEBUG=1 -DNOMINMAX -DIS_BUILD -DBUILDING_JSCONLY__ -DASSERT_ENABLED=0 -fvisibility=hidden -fvisibility-inlines-hidden  \
		-mmacosx-version-min=11.0 \
		-O1 -mtune=native \
		-fno-exceptions \
		-fno-rtti \
		-ferror-limit=1000 \
		 \
		-g3 -c -o src/bun.js/debug-bindings-obj/webcrypto/JSCryptoHmacKeyAlgorithm.o src/bun.js/bindings/webcrypto/JSCryptoHmacKeyAlgorithm.cpp
-- Looking for shm_open
In file included from src/bun.js/bindings/webcrypto/JSAesCbcCfbParams.cpp:25:
In file included from /Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/webcrypto/JSAesCbcCfbParams.h:26:
In file included from /Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/webcore/JSDOMConvertDictionary.h:28:
/Users/Vaughan/dev/fork/+bun/bun/src/bun.js/bindings/IDLTypes.h:39:10: fatal error: 'WebGLAny.h' file not found
#include "WebGLAny.h"
         ^~~~~~~~~~~~

@Jarred-Sumner
Copy link
Collaborator

#include "WebGLAny.h"

This flag means ENABLE(WEBGL) is being set when it shouldn't be

Is your cmakeconfig.h the one from bun or is it from an official build of webkit? It needs to be Bun's JSCOnly port

@Jarred-Sumner
Copy link
Collaborator

I think all the usages of {any} are incorrect

{any] prints the raw bytes as integers

We want strings

To fix:

/Users/jarred/zig/0.11.0-dev.182+14986077c/files/lib/std/fmt.zig:982:16: error: specifier 'x' has been deprecated, wrap your argument in std.fmt.fmtSliceHexLower instead
        'x' => @compileError("specifier 'x' has been deprecated, wrap your argument in std.fmt.fmtSliceHexLower instead"),
               ^

we need to wrap in std.fmt.fmtSliceHexLower

which is annoying

@vjpr
Copy link
Contributor Author

vjpr commented Nov 15, 2022

I think all the usages of {any} are incorrect

Yep its caused a few other issues too. It was the only global find+replace I did. Need to double check each usage individually.

@Jarred-Sumner
Copy link
Collaborator

@vjpr
Copy link
Contributor Author

vjpr commented Nov 16, 2022

Once I fix my build on Ventura I can finish it off.

@Jarred-Sumner
Copy link
Collaborator

I think the best approach would be to use stage2. They're about to delete stage1. But, they haven't implemented async/await support yet.

There are about 15 usages of suspend / resume in Bun.

I don't think it would be too difficult to replace them with function calls. Some kind of Promise-like abstraction.

The only tricky one to replace will be the @asyncCall inside Bun's transpiler. This is how macros work. They get their own stack space so that you don't need to worry about stack overflow due to running everything inside a highly recursive JS parser.

For that, I think we will need to use long jump in C. Which is fine. But it means we have to have a "suspend" and "resume" specifically for those to ensure that things like defer are run as expected and it will need some test coverage. We might also want this for HTMLRewriter later, too because that is re-entrant to the event loop in order to support stack-allocated data inside.

@vjpr
Copy link
Contributor Author

vjpr commented Nov 16, 2022

Is this because you think that the segfault from the stage1 build will be to hard to track down?

I'd say its probably better to get stage1 working if we can, and then upgrade stage2 separately...

@vjpr
Copy link
Contributor Author

vjpr commented Dec 7, 2022

NOTE:

we are planning to upgrade to zig's stage2 compiler this week and that will likely cut that requirement by 1/3

https://discord.com/channels/876711213126520882/888839314056839309/1046978389350817812

@vjpr
Copy link
Contributor Author

vjpr commented Dec 14, 2022

See: #1610

@vjpr vjpr closed this Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants