Repository navigation
Conversation
I'm not sure these messages ever printed until now.
Due to the @setCold(true) in Global.crash, this could improve performance as well.
This is part 1 of a new exit API that is supposed to make it harder to choose the wrong exit code and forgetting to mark paths as cold.
This finishes the new exit API.
ghost
changed the title
| } | ||
|
|
||
| /// Flushes output and exits otherwise unsuccessfully with a specific exit code. | ||
| pub fn exitOther(exit_code: u8) noreturn { |
There was a problem hiding this comment.
what do you think about renaming this to existCustom or exitCode to have more exact naming?
There was a problem hiding this comment.
I thought about using the word "custom" but "other" is shorter and not a less "exact" name IMO. It's the exit function you use for any other exit code or especially in any other case.
There was a problem hiding this comment.
It could also be named crashOther though.
I do think this API could still be improved. I still think an API consisting of exit(.failure), exit(.success), and exit(.{ .other = 50 }) would be better but I wasn't sure if @setCold(true) would work properly (it seems to mark the whole function as cold instead of only that one code path? I think this could be fixed by using functions within the one exit function and putting the @setCold(true)s there though) and I haven't received any feedback from @Jarred-Sumner yet.
There was a problem hiding this comment.
Ya I can agree on that, good point. But wouldn't making exist_code optional in exit more sens? so by default it would exist with 0 and if you pass any number in an argument it would use it 🤔
There was a problem hiding this comment.
@SerekKiri one problem with that is that Zig does not have optional parameters.
There was a problem hiding this comment.
Ok so I read wrong thread in zig repo then, nvm! No more comments 👍🏼
There was a problem hiding this comment.
Oh I didn't see the comment about crashOther but that's the best option I think with exist(.failure) etc. but I guess @Jarred-Sumner feedback would help here. I'm not experienced with zig yet so I can't help with that too much. 
|
For now it's probably best to close. This is low-priority anyway. |
ghost
closed this
oven-sh/WebKit#545 completes oven-sh/WebKit#155. NativeExecutable, builtin functions written in JavaScript, bound functions and remote functions now print `function name() { [native code] }`, the same one-line shape as InternalFunction constructors and V8. lodash's isNative() builds a RegExp from hasOwnProperty's source and tests Map against it. With one shape the RegExp matches, so cloneDeep, memoize, uniq and isEqual use a native Map instead of a linear-scan ListCache under Bun. cloneDeep of 8 000 objects: 1 971 ms before, linear after. The pin moves from the oven-sh/WebKit#541 preview to the fork's main at 01de4c1d, which also carries oven-sh/WebKit#529, #542 and #543. #543 moved the dynamic-import TLA deadlock check into the engine and removed JSModuleLoader::asyncEvaluationOrderForKey and the referrerAsyncOrder parameter of JSC::importModule, so moduleLoaderImportModule no longer computes or passes it. Tests: test/js/bun/jsc/function-prototype-tostring.test.ts and the oven-sh/WebKit#545 stress file as a jsc-stress fixture. Both fail on Bun 1.4.1.
1 participant
It's slightly different from what I described in the PR description of #535. Now
Global.exit()means successful exit,Global.crash()retains its meaning of unsuccessful exit, andGlobal.exitOther(n)is used for any other specific exit code, with the latter two having@setCold(true)set for hints to the compiler.While looking over the diff I notice that a lot of times the success-indicating
Global.exit()is used after an error was printed. It seems like it's not well defined when exactlyGlobal.crash()should be used and whenGlobal.exit().It'd probably be better if there was an error printing function that prints both a message and also exits with 1.