Skip to content

chore: modernize repo#67

Closed
dalloriam wants to merge 1 commit into
masterfrom
feat/modernize
Closed

chore: modernize repo#67
dalloriam wants to merge 1 commit into
masterfrom
feat/modernize

Conversation

@dalloriam

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Modernizes the Rust workspace by upgrading to Rust 2024 edition, refreshing key dependencies, and updating developer tooling/CI workflows.

Changes:

  • Bump all crates to Rust edition 2024, update workspace resolver, and refresh dependency versions.
  • Apply small Rust-idiomatic cleanups (e.g., div_ceil, Display, iterator simplifications).
  • Modernize GitHub Actions CI/coverage workflows and simplify just tasks.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
vm/src/stack.rs Removes redundant as usize casts around size_of usage.
vm/src/memutil.rs Uses div_ceil for alignment calculation.
vm/src/loader.rs Minor type cleanup plus lint suppression on Program.header.
vm/Cargo.toml Upgrade edition/deps (criterion/byteorder/log/snafu).
slang-cli/src/cli.rs Minor reference passing simplification.
slang-cli/Cargo.toml Upgrade edition/deps (anyhow/env_logger/log).
justfile Simplify linting to cargo clippy --workspace and format args.
instructor/src/instruction.rs Small iterator/borrow cleanup in operand writing.
instructor/Cargo.toml Upgrade edition and byteorder.
assembler/Cargo.toml Upgrade edition/deps (byteorder/log/snafu).
argot/src/main.rs Log error without allocating a new String.
argot/src/argot/syntax/operator.rs Replace ToString impl with Display.
argot/src/argot/compiler/second_pass.rs Simplify function key collection.
argot/src/argot/compiler/scope.rs Simplify PartialOrd to defer to Ord.
argot/src/argot/compiler/label.rs Use is_multiple_of and rename a local for clarity.
argot/src/argot/compiler/first_pass.rs Adds dead_code suppressions on fields.
argot/Cargo.toml Upgrade edition/deps (anyhow/clap/env_logger/log/snafu).
Cargo.toml Set workspace resolver = "3".
.github/workflows/coverage.yml Update actions; run tarpaulin directly; add rust-cache; install tarpaulin via cargo.
.github/workflows/ci.yml Update actions; add clippy component and rust-cache; run tests + clippy on push/PR.
.github/workflows/release.yml Removed tag-based release workflow.
Comments suppressed due to low confidence (4)

vm/src/loader.rs:34

  • Adding #[allow(dead_code)] on a public field makes it easy to miss genuinely-unused API/data and can mask regressions when the field is expected to be read. If the header is intentionally kept for future use, consider either making the field private with an accessor, or using it in code paths that need it, rather than suppressing the lint at the field level.
pub struct Program {
    #[allow(dead_code)]
    pub header: Header,
    pub ro_block: Vec<u8>,
    pub program_text: Vec<u8>,

.github/workflows/coverage.yml:22

  • cargo install cargo-tarpaulin without pinning a version (and without --locked) makes CI non-deterministic and can break unexpectedly when tarpaulin publishes a new release or dependency resolution changes. Prefer pinning a specific tarpaulin version and using --locked, or using an install action that supports version pinning/caching.
      - name: Install cargo-tarpaulin
        run: cargo install cargo-tarpaulin

      - name: Run cargo-tarpaulin
        run: cargo tarpaulin --out xml

.github/workflows/ci.yml:4

  • This PR deletes the only release.yml workflow and there is no replacement under .github/workflows/. If releases are still expected to be automated, this removal will stop tag-based validation/release jobs; consider keeping a release workflow (modernized to current actions) or documenting the new release process in-repo.
name: CI

on: [push, pull_request]

argot/src/argot/compiler/first_pass.rs:16

  • Using #[allow(dead_code)] to silence unused-field warnings on exported structs can hide real issues over time. If name/return_type are not part of the intended API, consider making them private (or removing them); if they are intended to be used later, consider adding the usage or a short comment explaining why the fields are retained.
pub struct FunctionDecl {
    #[allow(dead_code)]
    pub name: String,
    pub arguments: Vec<Argument>,
    #[allow(dead_code)]
    pub return_type: String,
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vm/src/loader.rs
Comment on lines 42 to 45
ensure!(
(header.ro_block_size + ELIS_HEADER_LENGTH) as usize <= data.len(),
(header.ro_block_size + ELIS_HEADER_LENGTH) <= data.len(),
ReadOnlySectionTooLongSnafu
);

Copilot AI Feb 25, 2026

Copy link

Choose a reason for hiding this comment

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

header.ro_block_size + ELIS_HEADER_LENGTH can overflow on 32-bit (and will panic in debug or wrap in release), which can bypass this length check and later cause slicing panics when parsing untrusted bytecode. Consider using checked_add (and failing with ReadOnlySectionTooLong) before comparing against data.len() and before computing the slice bounds.

Copilot uses AI. Check for mistakes.
@dalloriam dalloriam closed this Jul 8, 2026
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