ykjit / yk

yk packages
https://ykjit.github.io/yk/
Other
29 stars 7 forks source link

Rework codegen trait #1284

Closed ltratt closed 3 months ago

ltratt commented 3 months ago

The main part of the PR is https://github.com/ykjit/yk/commit/4988bf7705fe23f34b8ee65336baef15a3145b63.

vext01 commented 3 months ago

Out of interest, what's the expensive part of bringing up a codegen?

vext01 commented 3 months ago
15:13:46 error: methods called `new` usually return `Self`
15:13:46    --> ykrt/src/compile/jitc_yk/codegen/x86_64/mod.rs:102:5
15:13:46     |
15:13:46 102 | /     pub(crate) fn new() -> Result<Arc<dyn CodeGen>, Box<dyn Error>> {
15:13:46 103 | |         Ok(Arc::new(Self))
15:13:46 104 | |     }
15:13:46     | |_____^
15:13:46     |
15:13:46     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_ret_no_self
15:13:46     = note: `-D clippy::new-ret-no-self` implied by `-D warnings`
15:13:46     = help: to override `-D warnings` add `#[allow(clippy::new_ret_no_self)]`

That's a lint we could probably turn off globally in a config file?

ltratt commented 3 months ago

This is a place where you can see some unfortunate parts of type inference not working: you can't return the object directly but ? tricks the inferencer into casting to dyn T. Fixed in 43b01f3.

vext01 commented 3 months ago

please squash

ltratt commented 3 months ago

Squashed.