Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

PartialWitnessGenerator trait is not object safe #179

Closed
1 task done
TomAFrench opened this issue Apr 4, 2023 · 1 comment · Fixed by #180
Closed
1 task done

PartialWitnessGenerator trait is not object safe #179

TomAFrench opened this issue Apr 4, 2023 · 1 comment · Fixed by #180
Assignees
Labels
enhancement New feature or request

Comments

@TomAFrench
Copy link
Member

Problem

I'm in the process of writing the "nargo core" library which must allow passing in the backend which we want to use (for reasons discussed here). I'm trying to pass these as a trait object (i.e. Box<dyn PartialWitnessGenerator>) so the backend can be chosen in a different crate and passed in however this fails due to some methods which are making the trait non-object safe.

error[E0038]: the trait `acvm::Backend` cannot be made into an object
   --> crates/nargo/src/ops/execute.rs:7:18
    |
7   |     backend: Box<dyn Backend>,
    |                  ^^^^^^^^^^^ `acvm::Backend` cannot be made into an object
    |
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
   --> /home/tom/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/acvm-0.8.0/src/lib.rs:162:8
    |
162 |     fn solve_black_box_function_call(
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait cannot be made into an object because associated function `solve_black_box_function_call` has no `self` parameter
...
169 |     fn any_missing_assignment(
    |        ^^^^^^^^^^^^^^^^^^^^^^ the trait cannot be made into an object because associated function `any_missing_assignment` has no `self` parameter
...
182 |     fn solve_directives(
    |        ^^^^^^^^^^^^^^^^ the trait cannot be made into an object because associated function `solve_directives` has no `self` parameter

Proposed solution

We should either move these methods out of the trait (any_missing_assignments doesn't benefit from being a trait method rather than being a regular function) or add a self parameter.

Alternatives considered

No response

Additional context

No response

Submission Checklist

  • Once I hit submit, I will assign this issue to the Project Board with the appropriate tags.
@TomAFrench TomAFrench added the enhancement New feature or request label Apr 4, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Apr 4, 2023
@TomAFrench TomAFrench self-assigned this Apr 4, 2023
@TomAFrench TomAFrench moved this from 📋 Backlog to 🏗 In progress in Noir Apr 4, 2023
@TomAFrench
Copy link
Member Author

Just remembered that I can use impl PartialWitnessGenerator rather than Box<dyn PartialWitnessGenerator> in order to pass the backend as an argument. This means that this issue isn't a blocker, however I think that some of the refactoring in #180 would be nice to have outside of this so going to leave the PR (and this issue) open.

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Noir Apr 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant