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

refactor: ban std.debug.assert #10168

Merged
merged 17 commits into from
Apr 12, 2024
3 changes: 3 additions & 0 deletions .clangd
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Index:
Background: Skip # Disable slow background indexing of these files.

2 changes: 1 addition & 1 deletion .github/workflows/bun-linux-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ jobs:
mode: upsert
create_if_not_exists: false
message: |
✅ test failures on ${{ matrix.tag }} have been resolved.
✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.

<sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
- id: fail
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bun-mac-aarch64.yml
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ jobs:
mode: upsert
create_if_not_exists: false
message: |
✅ test failures on ${{ matrix.tag }} have been resolved.
✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.

<sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
- id: fail
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bun-mac-x64-baseline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ jobs:
mode: upsert
create_if_not_exists: false
message: |
✅ test failures on ${{ matrix.tag }} have been resolved.
✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.

<sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
- id: fail
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bun-mac-x64.yml
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ jobs:
mode: upsert
create_if_not_exists: false
message: |
✅ test failures on ${{ matrix.tag }} have been resolved.
✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.

<sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
- id: fail
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bun-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ jobs:
mode: upsert
create_if_not_exists: false
message: |
✅🪟 Test regressions on Windows ${{ matrix.arch }}${{ matrix.cpu == 'nehalem' && ' Baseline' || '' }} have been resolved.
✅🪟 Test regressions on Windows ${{ matrix.arch }}${{ matrix.cpu == 'nehalem' && ' Baseline' || '' }} have been resolved. Thank you.
- id: fail
name: Fail the build
if: steps.test.outputs.failing_tests != ''
Expand Down
86 changes: 86 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
name: lint

permissions:
contents: read

on:
workflow_dispatch:
pull_request:
push:
branches:
- main
- jarred/assert
paths:
- ".github/workflows/lint.yml"
- "src/**/*.zig"
- "src/*.zig"

jobs:
format:
name: lint
runs-on: ${{ vars.RUNNER_LINUX_X64 || 'ubuntu-latest' }}
if: github.repository_owner == 'oven-sh'
permissions: write-all
outputs:
text_output: ${{ steps.lint.outputs.text_output }}
json_output: ${{ steps.lint.outputs.json_output }}
count: ${{ steps.lint.outputs.count }}
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Setup Bun
uses: ./.github/actions/setup-bun
with:
bun-version: "1.1.3"
- name: Install Dependencies
run: |
bun --cwd=./packages/bun-internal-test install
- name: Lint
id: lint
run: |
bun ./packages/bun-internal-test/src/linter.ts || true
- uses: sarisia/actions-status-discord@v1
if: always() && steps.lint.outputs.text_output != '' && github.event_name == 'pull_request'
with:
title: ""
webhook: ${{ secrets.DISCORD_WEBHOOK }}
status: "failure"
noprefix: true
nocontext: true
description: |
Pull Request
### ❌ [${{github.event.pull_request.title}}](https://github.com/oven-sh/bun/pull/${{github.event.number}})

@${{ github.actor }}, there are ${{ steps.lint.outputs.count }} lint errors on ${{ github.ref_name }}

${{ steps.lint.outputs.text_output }}

**[View linter output](https://github.com/oven-sh/bun/actions/runs/${{github.run_id}})**
- name: Comment on PR
if: steps.lint.outputs.text_output != '' && github.event_name == 'pull_request'
uses: thollander/actions-comment-pull-request@v2
with:
comment_tag: lint-failures
message: |
❌ @${{ github.actor }} ${{ steps.lint.outputs.count }} lint errors

${{ steps.lint.outputs.text_output }}

**[View linter output](https://github.com/oven-sh/bun/actions/runs/${{github.run_id}})**

<sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
- name: Uncomment on PR
if: steps.lint.outputs.text_output == '' && github.event_name == 'pull_request'
uses: thollander/actions-comment-pull-request@v2
with:
comment_tag: lint-failures
mode: upsert
create_if_not_exists: false
message: |
✅ lint failures have been resolved. Thank you.

<sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
- id: fail
name: Fail the build
if: steps.lint.outputs.text_output != ''
run: exit 1
3 changes: 3 additions & 0 deletions packages/bun-internal-test/src/banned.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"std.debug.assert": "Use bun.assert instead"
}
68 changes: 68 additions & 0 deletions packages/bun-internal-test/src/linter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { $ } from "bun";
import BANNED from "./banned.json";
import * as action from "@actions/core";

const IGNORED_FOLDERS = [
// list of folders to ignore
"windows-shim",
];

const ci = !!process.env["GITHUB_ACTIONS"];
process.chdir(require("path").join(import.meta.dir, "../../../"));
let bad = [];
let report = "";
const write = (text: string) => {
process.stdout.write(text);
report += text;
};
for (const [banned, suggestion] of Object.entries(BANNED)) {
// Run git grep to find occurrences of std.debug.assert in .zig files
let stdout = await $`git grep -n "${banned}" "src/**/**.zig"`.text();

stdout = stdout.trim();
if (stdout.length === 0) continue;

let lines = stdout.split("\n");
// Parse each line to extract filename and line number
const matches = lines
.filter(line => !IGNORED_FOLDERS.some(folder => line.includes(folder)))
.map(line => {
const [path, lineNumber, ...text] = line.split(":");
return { path, lineNumber, banned, suggestion, text: text.join(":") };
});
// Check if we got any output
// Split the output into lines
if (matches.length === 0) continue;

write(`Banned **'${banned}'** found in the following locations:` + "\n");
matches.forEach(match => {
write(`${match.path}:${match.lineNumber}: ${match.text.trim()}` + "\n");
});
bad = bad.concat(matches);
}

if (report.length === 0) {
process.exit(0);
}

function link({ path, lineNumber, suggestion, banned }) {
action.error(`Lint failure: ${banned} is banned, ${suggestion}`, {
file: path,
startLine: Number(lineNumber),
endLine: Number(lineNumber),
});
return `[\`${path}:${lineNumber}\`](https://github.com/oven-sh/bun/blob/${process.env.GITHUB_SHA}/${path}#L${lineNumber})`;
}

if (ci) {
if (report.length > 0) {
action.setFailed(`${bad.length} lint failures`);
}
action.setOutput("count", bad.length);
action.setOutput("text_output", bad.map(m => `- ${link(m)}: ${m.banned} is banned, ${m.suggestion}`).join("\n"));
action.setOutput("json_output", JSON.stringify(bad));
action.summary.addRaw(report);
await action.summary.write();
}

process.exit(1);
4 changes: 2 additions & 2 deletions src/ArenaAllocator.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const std = @import("std");
const assert = std.debug.assert;
const assert = @import("root").bun.assert;
const mem = std.mem;
const Allocator = std.mem.Allocator;

Expand Down Expand Up @@ -132,7 +132,7 @@ pub const ArenaAllocator = struct {
self.child_allocator.rawFree(alloc_buf, align_bits, @returnAddress());
it = next_it;
} else null;
std.debug.assert(maybe_first_node == null or maybe_first_node.?.next == null);
assert(maybe_first_node == null or maybe_first_node.?.next == null);
// reset the state before we try resizing the buffers, so we definitely have reset the arena to 0.
self.state.end_index = 0;
if (maybe_first_node) |first_node| {
Expand Down
4 changes: 2 additions & 2 deletions src/StandaloneModuleGraph.zig
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ pub const StandaloneModuleGraph = struct {
var outfile_buf: bun.OSPathBuffer = undefined;
const outfile_slice = brk: {
const outfile_w = bun.strings.toWPathNormalized(&outfile_buf, std.fs.path.basenameWindows(outfile));
std.debug.assert(outfile_w.ptr == &outfile_buf);
bun.assert(outfile_w.ptr == &outfile_buf);
const outfile_buf_u16 = bun.reinterpretSlice(u16, &outfile_buf);
if (!bun.strings.endsWithComptime(outfile, ".exe")) {
// append .exe
Expand Down Expand Up @@ -652,7 +652,7 @@ pub const StandaloneModuleGraph = struct {
end -= offsets.byte_count;
@memcpy(to_read[0..offsets.byte_count], end[0..offsets.byte_count]);
if (comptime Environment.allow_assert) {
std.debug.assert(bun.strings.eqlLong(to_read, end[0..offsets.byte_count], true));
bun.assert(bun.strings.eqlLong(to_read, end[0..offsets.byte_count], true));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/StaticHashMap.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const mem = std.mem;
const math = std.math;
const testing = std.testing;

const assert = std.debug.assert;
const assert = @import("root").bun.assert;

pub fn AutoHashMap(comptime K: type, comptime V: type, comptime max_load_percentage: comptime_int) type {
return HashMap(K, V, std.hash_map.AutoContext(K), max_load_percentage);
Expand Down
20 changes: 0 additions & 20 deletions src/__global.zig
Original file line number Diff line number Diff line change
Expand Up @@ -162,26 +162,6 @@ pub fn panic(comptime fmt: string, args: anytype) noreturn {
}
}

// std.debug.assert but happens at runtime
pub fn invariant(condition: bool, comptime fmt: string, args: anytype) void {
if (!condition) {
_invariant(fmt, args);
}
}

inline fn _invariant(comptime fmt: string, args: anytype) noreturn {
@setCold(true);

if (comptime Environment.isWasm) {
Output.printErrorln(fmt, args);
Output.flush();
@panic(fmt);
} else {
Output.prettyErrorln(fmt, args);
Global.exit(1);
}
}

pub fn notimpl() noreturn {
@setCold(true);
Global.panic("Not implemented yet!!!!!", .{});
Expand Down
14 changes: 7 additions & 7 deletions src/allocators.zig
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ pub fn OverflowList(comptime ValueType: type, comptime count: comptime_int) type
}

pub fn append(block: *Block, value: ValueType) *ValueType {
if (comptime Environment.allow_assert) std.debug.assert(block.used < count);
if (comptime Environment.allow_assert) bun.assert(block.used < count);
const index = block.used;
block.items[index] = value;
block.used +%= 1;
Expand Down Expand Up @@ -155,9 +155,9 @@ pub fn OverflowList(comptime ValueType: type, comptime count: comptime_int) type
else
0;

if (comptime Environment.allow_assert) std.debug.assert(index.is_overflow);
if (comptime Environment.allow_assert) std.debug.assert(this.list.used >= block_id);
if (comptime Environment.allow_assert) std.debug.assert(this.list.ptrs[block_id].used > (index.index % count));
if (comptime Environment.allow_assert) bun.assert(index.is_overflow);
if (comptime Environment.allow_assert) bun.assert(this.list.used >= block_id);
if (comptime Environment.allow_assert) bun.assert(this.list.ptrs[block_id].used > (index.index % count));

return &this.list.ptrs[block_id].items[index.index % count];
}
Expand All @@ -168,9 +168,9 @@ pub fn OverflowList(comptime ValueType: type, comptime count: comptime_int) type
else
0;

if (comptime Environment.allow_assert) std.debug.assert(index.is_overflow);
if (comptime Environment.allow_assert) std.debug.assert(this.list.used >= block_id);
if (comptime Environment.allow_assert) std.debug.assert(this.list.ptrs[block_id].used > (index.index % count));
if (comptime Environment.allow_assert) bun.assert(index.is_overflow);
if (comptime Environment.allow_assert) bun.assert(this.list.used >= block_id);
if (comptime Environment.allow_assert) bun.assert(this.list.ptrs[block_id].used > (index.index % count));

return &this.list.ptrs[block_id].items[index.index % count];
}
Expand Down
2 changes: 1 addition & 1 deletion src/ast/base.zig
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ pub const Ref = packed struct(u64) {
}

pub fn initSourceEnd(old: Ref) Ref {
std.debug.assert(old.tag != .invalid);
bun.assert(old.tag != .invalid);
return init(old.inner_index, old.source_index, old.tag == .source_contents_slice);
}

Expand Down
Loading
Loading