Skip to content

Support file formatting and linting #38

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

Merged
merged 11 commits into from
Feb 1, 2025
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,35 @@ env:
CARGO_TERM_COLOR: always

jobs:
code-quality:
name: "Code Quality"
runs-on: [self-hosted, linux, normal]
steps:
- name: 'Check out code'
uses: actions/checkout@v4
with:
# Check out pull request HEAD instead of merge commit.
ref: ${{ github.event.pull_request.head.sha }}
submodules: recursive

- name: "Set up nightly Rust" # https://github.com/rust-lang/rustup/issues/3409
uses: dtolnay/rust-toolchain@master
with:
toolchain: nightly-2024-11-29 # Hardcoded version, same as is in the build.rs

- name: 'Build smir_pretty' # rustfmt documentation claims it is unstable on code that doesn't build
run: |
cargo build -vv

- name: "Check `cargo clippy`"
run: |
rustup component add clippy
cargo clippy -- -Dwarnings

- name: "Check `cargo fmt`"
run: |
rustup component add rustfmt
cargo fmt --check

integration-tests:
name: "Integration tests"
Expand All @@ -31,8 +60,8 @@ jobs:
toolchain: nightly-2024-11-29 # Hardcoded version, same as is in the build.rs

- name: 'Build smir_pretty'
run: |
cargo build -vv
run: | # Warning check should be redundant
RUSTFLAGS='--deny warnings' cargo build -vv
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this comment mean? Maybe it would make more sense to put --deny warnings in the build step for code-quality?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is, I think that cargo clippy should catch every error the compiler would have.It is redundant, should I just remove it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This made me realise that the integration-tests job wasn't dependent on code-quality. I think we normally do that and so I have changed it


- name: 'Run smir integration tests'
run: |
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ There are a few environment variables that can be set to control the tools outpu
2. `LINK_INST` - use a richer key-structure for the link-time `functions` map which uses keys that are pairs of a function type (`Ty`) _and_ an function instance kind (`InstanceKind`)
3. `DEBUG` - serialize additional data in the JSON file and dump logs to stdout

## Development

To ensure code quality, all code is required to pass `cargo clippy` and `cargo fmt` without warning to pass CI.

## Tests

### Running the Tests
Expand Down
10 changes: 5 additions & 5 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,30 @@ use std::process::Command;

fn main() {
let status = Command::new("rustup")
.args(&["install", "nightly-2024-11-29"])
.args(["install", "nightly-2024-11-29"])
.status()
.expect("build.rs failed to install nightly-2024-11-29");

println!("Installed nightly-2024-11-29: {}", status);

let status = Command::new("rustup")
.args(&["default", "nightly-2024-11-29"])
.args(["default", "nightly-2024-11-29"])
.status()
.expect("build.rs failed to default nightly-2024-11-29");

println!("Defaulted nightly-2024-11-29: {}", status);

let status = Command::new("rustup")
.args(&["component", "add", "rustc-dev"])
.args(["component", "add", "rustc-dev"])
.status()
.expect("build.rs failed to install rustc-dev");

println!("Added component rustc-dev: {}", status);

let status = Command::new("rustup")
.args(&["component", "add", "llvm-tools"])
.args(["component", "add", "llvm-tools"])
.status()
.expect("build.rs failed to install llvm-tools");

println!("Added component llvm-tools: {}", status);
}
}
20 changes: 8 additions & 12 deletions src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,32 @@
//!
//! However, we prefer a non-macro version for clarity and build simplicity.

extern crate rustc_middle;
extern crate rustc_driver;
extern crate rustc_interface;
extern crate rustc_smir;
extern crate rustc_middle;
extern crate rustc_session;
use rustc_middle::ty::TyCtxt;
extern crate rustc_smir;
use rustc_driver::Compilation;
use rustc_interface::interface::Compiler;
use rustc_middle::ty::TyCtxt;
use rustc_smir::rustc_internal;

struct StableMirCallbacks {
callback_fn: fn (TyCtxt) -> (),
callback_fn: fn(TyCtxt) -> (),
}

impl rustc_driver::Callbacks for StableMirCallbacks {
fn after_analysis<'tcx>(
&mut self,
_compiler: &Compiler,
tcx: TyCtxt<'tcx>,
) -> Compilation {

fn after_analysis(&mut self, _compiler: &Compiler, tcx: TyCtxt) -> Compilation {
let _ = rustc_internal::run(tcx, || (self.callback_fn)(tcx));

Compilation::Continue
}
}

pub fn stable_mir_driver(args_outer: &Vec<String>, callback_fn: fn (TyCtxt) -> ()) {
pub fn stable_mir_driver(args_outer: &[String], callback_fn: fn(TyCtxt) -> ()) {
let mut callbacks = StableMirCallbacks { callback_fn };
let early_dcx = rustc_session::EarlyDiagCtxt::new(rustc_session::config::ErrorOutputType::default());
let early_dcx =
rustc_session::EarlyDiagCtxt::new(rustc_session::config::ErrorOutputType::default());
rustc_driver::init_rustc_env_logger(&early_dcx);
let _ = rustc_driver::RunCompiler::new(args_outer, &mut callbacks).run();
}
6 changes: 2 additions & 4 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ fn main() {
let mut args: Vec<String> = env::args().collect();

match args.get(1) {
None =>
stable_mir_driver(&args, emit_smir), // backward compatibility
None => stable_mir_driver(&args, emit_smir), // backward compatibility
Some(arg) if arg == "--json" => {
args.remove(1);
stable_mir_driver(&args, emit_smir)
Expand All @@ -20,7 +19,6 @@ fn main() {
args.remove(1);
stable_mir_driver(&args, emit_dotfile)
}
Some(_other) =>
stable_mir_driver(&args, emit_smir), // backward compatibility
Some(_other) => stable_mir_driver(&args, emit_smir), // backward compatibility
}
}
Loading