Skip to content

Remove dead code from bun_core: the function form of pretty_errorln - #47084

Open
robobun wants to merge 1 commit into
mainfrom
robobun/d52ee154/dead-code-sweep
Open

robobun wants to merge 1 commit into
mainfrom
robobun/d52ee154/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • Delete the function and the clippy.toml row that names it: 14 lines in 2 files. Narrow crate-internal Rust visibility across all targets and delete the code it proves dead #36184 did the same for pretty_error, note and debug.
  • Correct because the only other mentions left are use lines that import the macro.
  • Verified: bun bd, bun run rust:check-all (12 targets), cargo check --workspace --all-targets, bun run rust:clippy.
  • Self-reviewed: 47 concerns raised, 40 addressed, most by a cut of the first draft from 122 lines to 14. Rejected (Notes): a refile of the open sweeps that conflict with main, edits to the pull requests below, and three separate changes.

Background

  • Each print helper in bun_core::output has a macro form and a function form. The macro rewrites <tag> markup at compile time. clippy.toml lists the function form under disallowed-methods.
  • If the row stays and the function goes, the path resolves to the macro and clippy prints expected a function, found a macro.

Downsides

Behaviour change: none

Notes

Removed

  • pub fn pretty_errorln(payload: impl PrettyFmtInput) in src/bun_core/output.rs, with its doc comment and #[inline].
  • The bun_core::output::pretty_errorln row of disallowed-methods in clippy.toml.
  • git grep pretty_errorln, without the lines that contain pretty_errorln!, finds only use lines, the macro_rules! line and one comment. rg over build/debug/codegen finds nothing.
  • cargo clippy -p bun_ptr --no-deps prints clippy.toml:28:12: warning: expected a function, found a macro when the row stays. It prints no such line on this branch.

What earlier sweeps said about this item

Open pull requests

Found dead in this run, and left alone

How the candidates were found

  • A relink of the debug build with -Wl,--gc-sections,--print-gc-sections, with each discarded function mapped to its source definition.
  • Rust: about 1,050 functions and trait impls with no live symbol were deleted in a scratch state. cargo check for linux, windows and macos, then the unit tests, put all but two back.
  • C and C++: clang with -Wunused-function -Wunused-template -Wunused-member-function -Wunused-macros over 603 translation units (20 did not parse alone). Each real hit is in an open pull request.
  • TypeScript: tsc --noUnusedLocals over src/js.

Rejected from the self-review

Tests, debug build of this branch

  • test/cli/bun.test.ts, test/cli/run/commonjs-invalid.test.ts, test/cli/run/run-eval.test.ts: all pass.

bun_core::output::pretty_errorln (the function) has no caller. Its last
callers went away in #46021. The macro pretty_errorln! stays.

The clippy.toml row that names the function goes with it. Without the
function the row resolves to the macro and clippy warns
"expected a function, found a macro".
@robobun

robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:55 AM PT - Oct 27th, 2026

✅ @robobun, your commit 4353d5ce5cda85c4899a206cdf4cace1cae7d030 passed in Build #133764! 🎉


🧪   To try this PR locally:

bunx bun-pr 47084

That installs a local version of the PR into your bun-47084 executable, so you can run:

bun-47084 --bun

@robobun

robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 9b8347a and 4353d5c.

Files selected for processing: 2
  • clippy.toml
  • src/bun_core/output.rs
Files with no reviewable changes: 2
  • clippy.toml
  • src/bun_core/output.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

The public function-form pretty_errorln API and its clippy.toml disallowed-method entry were removed. Other output-method restrictions remain unchanged.

Changes

pretty_errorln API removal

Layer / File(s) Summary
Remove function API
src/bun_core/output.rs, clippy.toml
The public pretty_errorln function and its disallowed-method entry were removed.

Suggested reviewers: alii

Priority: Low

Merge Risk: Minimal · up to 4353d

The removal has no identified impact on current repository callers and is ready to merge after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the removed item and its scope: the function form of pretty_errorln in bun_core. The diff removes that function and its clippy.toml entry, so the title matches the main change. I…
Description check Passed The description follows the template's intent. It states the problem, the fix, and how the change was verified (bun bd, bun run rust:check-all, cargo check --workspace --all-targets, `bun run ru…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the removed item and its scope: the function form of pretty_errorln in bun_core. The diff removes that function and its clippy.toml entry, so the title matches the main change. It does not mention the clippy.toml edit, which is a minor omission.

Full details: Description check

Explanation

The description follows the template's intent. It states the problem, the fix, and how the change was verified (bun bd, bun run rust:check-all, cargo check --workspace --all-targets, bun run rust:clippy, and three named test files). It also gives background, downsides, and a notes section. The template headings What does this PR do? and How did you verify your code works? are not used as written. The verification section lists commands and results, so the content is present under different headings.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward dead-code removal with no behaviour change.

What was reviewed:

  • Searched the whole checkout for function-form callers (pretty_errorln( and output::pretty_errorln) — none remain; the only hit is a pretty_errorln! macro call in src/runtime/server/HTMLBundle.rs.
  • The use bun_core::{..., pretty_errorln} imports in src/output/lib.rs and src/runtime/cli/*.rs, and use crate::{pretty, pretty_error, pretty_errorln} at src/bun_core/output.rs:15, all resolve to the #[macro_export] macro (defined at output.rs:2153), which is still used inside output.rs (warn/debug_warn at lines 174/183), so nothing becomes orphaned.
  • Removing the clippy.toml disallowed-methods row is required, not optional: with the function gone the path would resolve to the macro and clippy would warn on the config.
  • src/CLAUDE.md only documents the macro form, so no docs describe the removed state.
Extended reasoning...

The change deletes the unused pub fn pretty_errorln function form from src/bun_core/output.rs and the matching disallowed-methods entry in clippy.toml; 14 lines removed, nothing added. It touches no security-sensitive surface. A repo-wide search confirms no remaining function-form callers and that every remaining pretty_errorln name resolves to the retained macro, and neither file is covered by .github/CODEOWNERS. The bug hunt exited on dry_streak with no findings, and the three ruled-out candidates all reduce to the same verified fact that only the macro is referenced.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants