Add `no_std` Support #374

bushrat011899 · 2025-03-03T10:17:06Z

Objective

Add no_std support to codespan-reporting, allowing its use in initiatives such as wasm32v1-none support in wgpu.

Solution

Feature-gated termcolor, as it does not have no_std support
Added #![no_std] to all 3 crates. Noting that codespan-lsp does not have no_std support as lsp-types does not haveno_std support.
Added std and termcolor features to codespan-reporting and codespan, which are both enabled by default.
Various minor changes as suggested by clippy

Notes

This is my first time contributing to this project. I have attempted to split my changes into meaningful commits with descriptions as described in the CONTRIBUTING.md document. Please let me know if there's anything I can do to assist with reviewing and merging this PR.

This is effectively required for `no_std` support, as without it, dev-dependencies will enable features on regular dependencies, making it impossible to have `std` tests in `no_std` crates.

These clippy lints highlight cases where an import is referencing `std` or `alloc` when it instead could import from `alloc` or `core`. This helps expose the "true" extent of `std` usage in a crate.

`termcolor` is incompatible with `no_std`, but `codespan` and `codespan-reporting` have valid use even without this crate. Making it optional allows `no_std` to be entirely decided by the contents of the codespan crates themselves. Serde is also `no_std` compatible, as long as we disable default features.

During testing, `unicode-width` caused `cargo msrv find` to report `codespan-reporting`'s MSRV was 1.66, violating the current MSRV of 1.46. Since this is already the case, and there are features in 1.67, such as ilog10, which are desired within this crate, I have increased the MSRV to 1.67.

Before making substantive changes, `clippy` reported that these implementations for default can be derived. I have done so to silence the lint.

`Span::from_str` shadows `FromStr::from_str`. Instead of breaking the public API, I added an explicit `allow`

Caught by clippy, and actioned to reduce warnings

`Renderer::render_snippet_source` has too many arguments. Instead of breaking the public API, I'm suppressing this lint.

These double-references are automatically dereferenced anyway.

Increasing MSRV allows using the new `ilog10` method, solving a long-standing TODO

`ToString::to_string()` is preferred by `clippy` for converting `str` to `String`

`assert_eq` on a boolean is linted against by `clippy`, suggesting to use `assert!` instead.

The `Styles` type is only needed for controlling colour, which is only needed with `termcolor`. Since we only deal with UTF8 strings, `std::fmt::Write` is equivalent to `WriteColor` when colour isn't required.

Switch imports to relying on `core` and `alloc`. `std::io::Write` can be replaced by `core::fmt::Write` since we deal exclusively with UTF8 strings.

This checks for `no_std` compatibility on 3 key targets: - `x86_64-unknown-none`: represents desktop `no_std`, such as in kernels - `wasm32v1-none`: represents Web builds using the most broadly support WASM target -`thumbv6m-none-eabi`: represents typical embedded targets, such as the Raspberry Pi Pico Testing on all three platforms ensures the widest possible compatibility.

kaleidawave · 2025-03-03T11:50:22Z

Thanks for the change, would be good if this crate can work in more places!

(I need to update the CI, ignore the other errors) but I think cargo fmt does need running on this PR.

Will review the code before merging later this week ~ Thursday/Friday.

- `cargo fmt` - Fix typo in `ci.yml` which misused the matrix

bushrat011899 · 2025-03-03T22:05:21Z

Ran cargo fmt and also fixed a typo I had in the new check-no-std CI task I wrote (matrix wasn't enumerating properly).

kaleidawave · 2025-03-07T08:14:12Z

Ran cargo fmt and also fixed a typo I had in the new check-no-std CI task I wrote (matrix wasn't enumerating properly).

Thanks!

Will double check and merge later. I don't know whether the CI issue is something from this change or something generally related to CI of which I should handle in #373

kaleidawave

Looks good!

I think the WASM CI needs some changes as it is currently showing error: component 'rust-std' for target 'wasm32v1-none' is unavailable for download for channel '1.82.0'.

Once that is resolved and the new job is green I will merge.

I have mentioned a few other things that you can (but don't necessarily need to) amend.

kaleidawave · 2025-03-07T18:49:26Z

codespan-lsp/CHANGELOG.md

@@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ## [Unreleased]

-The minimum supported rustc version is now `1.46.0` (was `1.40.0`).
+The minimum supported rustc version is now `1.67.0` (was `1.40.0`).


I think this is a reasonable demand in 2025 👍

Yeah I think it's been long enough haha!

.github/workflows/ci.yml

kaleidawave · 2025-03-07T18:54:30Z

codespan-reporting/Cargo.toml

@@ -26,5 +27,13 @@ rustyline = "6"
 unindent = "0.1"

 [features]
+default = ["std", "termcolor"]


Nice, adding default should not cause breaking changes (unless a dependent specified no-default-features which would be pointless)

Yeah it's still technically a breaking change semantically, so you'll want to bump the version number for a release, but it's the nice kind of breaking change where migrating should be just enabling these features.

codespan-reporting/Cargo.toml

codespan-reporting/src/term/renderer.rs

kaleidawave · 2025-03-07T19:00:18Z

codespan-reporting/src/term/views.rs

-    // Use a saturating_add because in that edge case the number of digits
-    // will not be changed.
-    (n.saturating_add(1) as f64).log10().ceil() as usize
+    n.ilog10() as usize + 1


I see that rust-lang/rust#70887 was resolved. I think this is okay to be sneaked into this pr 👍

Yeah without this change the MSRV is 1.66, and with it's 1.67, so I figured why not, I'm already bumping the MSRV, and it actually solves a problem too, since no_std removes some of the floating point operations like log10!

codespan/src/index.rs

Co-Authored-By: Ben <[email protected]>

`wasm32v1-none` was only added in 1.84, so I've set that as the earliest Rust version used for testing `no_std` compatibility. Note that it's typical for embedded development to be done on nightly anyway, so `no_std` will usually happen on the very latest version of Rust.

bushrat011899 · 2025-03-08T10:00:24Z

Conflicts resolved and review items addressed. I believe the CI task should pass now too!

Not currently used and is incompatible with `thumbv6m-none-eabi`

bushrat011899 · 2025-03-08T10:34:58Z

Fixed a minor issue with thumbv6m-none-eabi and formatting from the upstream merge

Co-Authored-By: Ben <[email protected]>

kaleidawave · 2025-03-08T10:46:11Z

Stable check, test, format all work as well on these new targets!

Thanks for the quick updates. Anything else before merge?

Once merged, I will hopefully have a release within the next 7 days. Just waiting for some publishing keys from the owner of the repository :)

bushrat011899 · 2025-03-08T10:50:02Z

Nope, I think this should be good to go! Thanks for being so receptive to the changes. Looking forward to the new release!

bushrat011899 added 15 commits March 3, 2025 17:17

Switch to edition 2021 and resolver 2

8e1a6e5

This is effectively required for `no_std` support, as without it, dev-dependencies will enable features on regular dependencies, making it impossible to have `std` tests in `no_std` crates.

Add useful no_std lints

2486df2

These clippy lints highlight cases where an import is referencing `std` or `alloc` when it instead could import from `alloc` or `core`. This helps expose the "true" extent of `std` usage in a crate.

Switch to derived Default

4babadc

Before making substantive changes, `clippy` reported that these implementations for default can be derived. I have done so to silence the lint.

Suppress clippy::should_implement_trait

c05aad0

`Span::from_str` shadows `FromStr::from_str`. Instead of breaking the public API, I added an explicit `allow`

Remove redundant into() calls

59c3075

Caught by clippy, and actioned to reduce warnings

Suppress clippy::too_many_arguments

68b6071

`Renderer::render_snippet_source` has too many arguments. Instead of breaking the public API, I'm suppressing this lint.

Remove redundant references

82c96b2

These double-references are automatically dereferenced anyway.

Use ilog10 in count_digits

b899925

Increasing MSRV allows using the new `ilog10` method, solving a long-standing TODO

Use of format is redundant

1d0bac9

`ToString::to_string()` is preferred by `clippy` for converting `str` to `String`

Simplify assert_eq! statements

3e333fa

`assert_eq` on a boolean is linted against by `clippy`, suggesting to use `assert!` instead.

Implement termcolor feature gating

95f5294

The `Styles` type is only needed for controlling colour, which is only needed with `termcolor`. Since we only deal with UTF8 strings, `std::fmt::Write` is equivalent to `WriteColor` when colour isn't required.

Add no_std support

af7c140

Switch imports to relying on `core` and `alloc`. `std::io::Write` can be replaced by `core::fmt::Write` since we deal exclusively with UTF8 strings.

bushrat011899 mentioned this pull request Mar 3, 2025

Support no_std where possible (specifically for wasm32v1-none) gfx-rs/wgpu#6826

Open

kaleidawave added the enhancement New feature or request label Mar 3, 2025

Fixes from CI run

584f995

- `cargo fmt` - Fix typo in `ci.yml` which misused the matrix

kaleidawave requested changes Mar 7, 2025

View reviewed changes

bushrat011899 and others added 4 commits March 8, 2025 20:42

Switch from underscore to allow(unused)

0bf71d2

Co-Authored-By: Ben <[email protected]>

Document clippy lints

128a6b2

Co-Authored-By: Ben <[email protected]>

Merge remote-tracking branch 'upstream/master' into no_std

1130af9

bushrat011899 requested a review from kaleidawave March 8, 2025 09:59

bushrat011899 added 2 commits March 8, 2025 21:29

Format

049900f

Remove serde/rc from codespan-reporting/serialization

4382b81

Not currently used and is incompatible with `thumbv6m-none-eabi`

This comment was marked as outdated.

Sign in to view

Test all combinations before giving up on check-no-std

b26948f

Co-Authored-By: Ben <[email protected]>

kaleidawave merged commit 0f2450a into brendanzab:master Mar 8, 2025
21 of 24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `no_std` Support #374

Add `no_std` Support #374

bushrat011899 commented Mar 3, 2025

kaleidawave commented Mar 3, 2025 •

edited

Loading

bushrat011899 commented Mar 3, 2025

kaleidawave commented Mar 7, 2025

kaleidawave left a comment

kaleidawave Mar 7, 2025

bushrat011899 Mar 8, 2025

kaleidawave Mar 7, 2025

bushrat011899 Mar 8, 2025

kaleidawave Mar 7, 2025

bushrat011899 Mar 8, 2025

bushrat011899 commented Mar 8, 2025

bushrat011899 commented Mar 8, 2025

This comment was marked as outdated.

kaleidawave commented Mar 8, 2025 •

edited

Loading

bushrat011899 commented Mar 8, 2025

Add no_std Support #374

Add no_std Support #374

Conversation

bushrat011899 commented Mar 3, 2025

Objective

Solution

Notes

kaleidawave commented Mar 3, 2025 • edited Loading

bushrat011899 commented Mar 3, 2025

kaleidawave commented Mar 7, 2025

kaleidawave left a comment

Choose a reason for hiding this comment

kaleidawave Mar 7, 2025

Choose a reason for hiding this comment

bushrat011899 Mar 8, 2025

Choose a reason for hiding this comment

kaleidawave Mar 7, 2025

Choose a reason for hiding this comment

bushrat011899 Mar 8, 2025

Choose a reason for hiding this comment

kaleidawave Mar 7, 2025

Choose a reason for hiding this comment

bushrat011899 Mar 8, 2025

Choose a reason for hiding this comment

bushrat011899 commented Mar 8, 2025

bushrat011899 commented Mar 8, 2025

This comment was marked as outdated.

kaleidawave commented Mar 8, 2025 • edited Loading

bushrat011899 commented Mar 8, 2025

Add `no_std` Support #374

Add `no_std` Support #374

kaleidawave commented Mar 3, 2025 •

edited

Loading

kaleidawave commented Mar 8, 2025 •

edited

Loading