Skip to content

Add explicit dependency on libfuzzer-sys#96

Merged
cfallin merged 1 commit intobytecodealliance:mainfrom
alexcrichton:fix-fuzz-build
Oct 21, 2022
Merged

Add explicit dependency on libfuzzer-sys#96
cfallin merged 1 commit intobytecodealliance:mainfrom
alexcrichton:fix-fuzz-build

Conversation

@alexcrichton
Copy link
Copy Markdown
Member

This fixes a build issue with the most recent version of libfuzzer-sys where the prior version of the fuzz_target! macro didn't need the crate as an explicit dependency but the latest version does.

This fixes a build issue with the most recent version of `libfuzzer-sys`
where the prior version of the `fuzz_target!` macro didn't need the
crate as an explicit dependency but the latest version does.
@alexcrichton
Copy link
Copy Markdown
Member Author

The fuzz build also currently prints out:

warning: for loop over a `Result`. This is more readably written as an `if let` statement
  --> fuzz_targets/moves.rs:65:18
   |
65 |         for i in u.int_in_range(0..=2) {
   |                  ^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
   |
65 |         while let Ok(i) = u.int_in_range(0..=2) {
   |         ~~~~~~~~~~~~~ ~~~
help: consider using `if let` to clear intent
   |
65 |         if let Ok(i) = u.int_in_range(0..=2) {
   |         ~~~~~~~~~~ ~~~

which is just a warning so not fatal, but I wasn't sure what the intended code here was so I figured I'd ask.

Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm taking a look at the warning and will address in another PR -- it seems pretty clearly was meant to be for i in 0..limit and the iterability of Result masked the lack of range (yikes!).

@cfallin cfallin merged commit d6b1509 into bytecodealliance:main Oct 21, 2022
@jameysharp
Copy link
Copy Markdown
Contributor

What libfuzzer-sys change caused this to be necessary? I'd expect this to be a bug in that crate, not something that should be worked around here. I think libfuzzer-sys 0.4.3 should have made this unnecessary, so I'd like to know what broke after that.

In #69 I removed this dependency because it requires keeping dependencies in sync across multiple Cargo.toml files. So I'd like to revert this PR but clearly we need something.

@alexcrichton alexcrichton deleted the fix-fuzz-build branch October 24, 2022 19:26
@alexcrichton
Copy link
Copy Markdown
Member Author

I'm not sure what the change was, I mostly just wanted to fix the fuzz build, I didn't investigate much futher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants