-
Notifications
You must be signed in to change notification settings - Fork 824
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
Set compiler config to be owned (following wasm-c-api) #1874
Conversation
bors try |
tryBuild failed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
@@ -207,8 +207,8 @@ impl CompilerConfig for LLVM { | |||
} | |||
|
|||
/// Transform it into the compiler. | |||
fn compiler(&self) -> Box<dyn Compiler + Send> { | |||
Box::new(LLVMCompiler::new(&self)) | |||
fn compiler(self: Box<Self>) -> Box<dyn Compiler + Send> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using From
/Into
traits here; not sure how this is used but that could be a bit cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried, couldn't make it work with From
. We can do it in a PR apart if necessary
bors r+ |
1874: Set compiler config to be owned (following wasm-c-api) r=syrusakbary a=syrusakbary <!-- Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test: https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests --> # Description Set compiler config to be owned (following wasm-c-api) <!-- Provide details regarding the change including motivation, links to related issues, and the context of the PR. --> # Review - [ ] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Syrus <[email protected]> Co-authored-by: Syrus Akbary <[email protected]>
Build failed: |
Merging directly to alleviate CI. Tests are passing locally |
Description
Set compiler config to be owned (following wasm-c-api)
Review