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

Rust 1.73 updates #298

Merged
merged 2 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion bril-rs/bril2json/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ keywords = ["compiler", "bril", "parser", "data-structures", "language"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
clap = { version = "4.3", features = ["derive"] }
clap = { version = "4.4", features = ["derive"] }
Copy link
Owner

Choose a reason for hiding this comment

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

It's obviously not important now, but it occurs to me that we could consider setting up a Cargo workspace to centralize dependency versions like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can, but there are two usability issues as far as I can tell.

One I knew about previously is that if you run a cargo command at the workspace level(compared to the crate level) then it will try to union all the possible feature flags of each dependency together. This is incompatible with some of the projects that only expect a subset of the bril-rs flags(a bril* tool that doesn't expect bril-rs to be built with positions for instance).

One minor thing I just figured out is that there are other settings that previously local would be forced to be workspace wide. Specifically for brillvm it is important to have panic = "abort" lto = true set for building/linking the no_std rt library. With a workspace, this gets forced upon all of the projects which isn't ideal(but not breaking/work around-able in this case I think). rust-lang/cargo#8264 (comment)

Copy link
Owner

Choose a reason for hiding this comment

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

Makes sense! That first argument seems like sufficient reason not to do this (since bril-rs is meant to be a very configurable crate).

lalrpop-util = { version = "0.20", features = ["lexer"] }
regex = "1.9"

Expand Down
1 change: 1 addition & 0 deletions bril-rs/bril2json/src/bril_grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#![allow(clippy::uninlined_format_args)]
#![allow(clippy::match_same_arms)]
#![allow(clippy::option_if_let_else)]
#![allow(clippy::extra_unused_lifetimes)]

use std::str::FromStr;
use std::path::PathBuf;
Expand Down
2 changes: 1 addition & 1 deletion bril-rs/brild/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ keywords = ["compiler", "bril", "parser", "data-structures", "language"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
clap = { version = "4.3", features = ["derive"] }
clap = { version = "4.4", features = ["derive"] }
thiserror = "1.0"

[dependencies.bril2json]
Expand Down
2 changes: 1 addition & 1 deletion bril-rs/brillvm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ default-run = "main"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
clap = { version = "4.3", features = ["derive"] }
clap = { version = "4.4", features = ["derive"] }
inkwell = { version = "0.2", features = ["llvm16-0"] }
libc-print = "0.1"

Expand Down
12 changes: 6 additions & 6 deletions bril-rs/brillvm/src/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl Fresh {
fn build_op<'a, 'b>(
context: &'a Context,
builder: &'a Builder,
heap: &mut Heap<'a, 'b>,
heap: &Heap<'a, 'b>,
fresh: &mut Fresh,
op: impl Fn(Vec<BasicValueEnum<'a>>) -> BasicValueEnum<'a>,
args: &'b [String],
Expand All @@ -160,7 +160,7 @@ fn build_op<'a, 'b>(
fn build_effect_op<'a, 'b>(
context: &'a Context,
builder: &'a Builder,
heap: &mut Heap<'a, 'b>,
heap: &Heap<'a, 'b>,
fresh: &mut Fresh,
op: impl Fn(Vec<BasicValueEnum<'a>>),
args: &'b [String],
Expand Down Expand Up @@ -190,7 +190,7 @@ fn build_instruction<'a, 'b>(
context: &'a Context,
module: &'a Module,
builder: &'a Builder,
heap: &mut Heap<'a, 'b>,
heap: &Heap<'a, 'b>,
block_map: &mut HashMap<String, BasicBlock<'a>>,
llvm_func: FunctionValue<'a>,
fresh: &mut Fresh,
Expand Down Expand Up @@ -1247,7 +1247,7 @@ pub fn create_module_from_program<'a>(
// Now actually build each function
funcs
.into_iter()
.for_each(|(llvm_func, instrs, mut block, mut heap)| {
.for_each(|(llvm_func, instrs, mut block, heap)| {
let mut last_instr = None;

// If their are actually instructions, proceed
Expand Down Expand Up @@ -1284,7 +1284,7 @@ pub fn create_module_from_program<'a>(
context,
&runtime_module,
&builder,
&mut heap,
&heap,
&mut block_map,
llvm_func,
&mut fresh,
Expand Down Expand Up @@ -1375,7 +1375,7 @@ pub fn create_module_from_program<'a>(
build_effect_op(
context,
&builder,
&mut heap,
&heap,
&mut fresh,
|v| {
builder.build_call(
Expand Down
2 changes: 1 addition & 1 deletion bril-rs/rs2bril/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ keywords = ["compiler", "bril", "parser", "data-structures", "language"]
[dependencies]
syn = {version = "2.0", features = ["full", "extra-traits"]}
proc-macro2 = {version = "1.0", features = ["span-locations"]}
clap = { version = "4.3", features = ["derive"] }
clap = { version = "4.4", features = ["derive"] }

[dependencies.bril-rs]
version = "0.1.0"
Expand Down
31 changes: 18 additions & 13 deletions brilirs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,34 @@ readme = "README.md"
repository = "https://github.com/sampsyo/bril"
# license = "MIT"
license-file = "../LICENSE"
categories = ["command-line-utilities", "compilers", "data-structures", "parser-implementations"]
categories = [
"command-line-utilities",
"compilers",
"data-structures",
"parser-implementations",
]
keywords = ["compiler", "bril", "interpreter", "data-structures", "language"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[build-dependencies]
clap = { version = "4.3", features = ["derive"] }
clap_complete= { version = "4.3", optional = true }
clap = { version = "4.4", features = ["derive"] }
clap_complete = { version = "4.4", optional = true }

[dependencies]
thiserror = "1.0"
clap = { version = "4.3", features = ["derive"] }
fxhash = "0.2"
mimalloc = "0.1"
itoa = "1.0"
thiserror = "1.0"
clap = { version = "4.4", features = ["derive"] }
fxhash = "0.2"
mimalloc = "0.1"
itoa = "1.0"

[dependencies.bril-rs]
version = "0.1.0"
path = "../bril-rs"
features = ["ssa", "memory", "float", "speculate", "char"]
version = "0.1.0"
path = "../bril-rs"
features = ["ssa", "memory", "float", "speculate", "char"]

[dependencies.bril2json]
version = "0.1.0"
path = "../bril-rs/bril2json"
version = "0.1.0"
path = "../bril-rs/bril2json"

[profile.release]
# this can shave off a few ms but doubles the build time so it's not really worth it
Expand Down
2 changes: 1 addition & 1 deletion brilirs/src/interp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ pub fn execute_main<T: std::io::Write, U: std::io::Write>(
writeln!(profiling_out, "total_dyn_inst: {}", state.instruction_count)
// We call flush here in case `profiling_out` is a https://doc.rust-lang.org/std/io/struct.BufWriter.html
// Otherwise we would expect this flush to be a nop.
.and_then(|_| profiling_out.flush())
.and_then(|()| profiling_out.flush())
.map_err(InterpError::IoError)?;
}

Expand Down