Added 'add_fuel' command line option

[ilikepi63]

Relatively new to both Rust and Wasmtime, so trying to just pick up a small first issue here.

PR to try a solution to Issue #3717

Currently just adds a "add_fuel" config option to the wasmtime API. I was taking a look and I think I might need to update this default value when the interpreter is instantiated:

    pub fn new(state: InterpreterState<'a>) -> Self {
        Self { state, fuel: None }
    }

to

    pub fn new(state: InterpreterState<'a>, fuel: Option<u64> = None) -> Self {
        Self { state, fuel: fuel }
    }

Don't currently have any test cases for the specific command line option just yet. Just wanted to check if I was on the right track.

CC: @fitzgen

[fitzgen]

You don't need to modify the clif interpreter at all here.

You want to add a call to https://docs.rs/wasmtime/latest/wasmtime/struct.Store.html#method.add_fuel somewhere around here: https://github.com/bytecodealliance/wasmtime/blob/main/src/commands/run.rs#L165

[ilikepi63]

@fitzgen Thank you so much! That specific instantiation eluded me!

Thank you so much for being so patient and understanding, as well as holding my hand through this!

[a1phyr]

Maybe we could warn the user if --add-fuel is used without --consume-fuel ?

[a1phyr]

I don't think you want to panic here, even though this should never fail.

Suggested change

	store.add_fuel(self.common.add_fuel).unwrap();
	store.add_fuel(self.common.add_fuel)?;

[fitzgen]

Thanks!

We can use clap's requires functionality to require that both --consume-fuel and --add-fuel are given if either one is given. This avoids having to check and warn (or at least pushes that into clap rather than here).

---

But backing up a bit, the fact that users always have to path both is a hint that maybe this is one knob, not two different knobs.

What if instead we removed --consume-fuel and --add-fuel <N> and instead had just --fuel <N>.

#[structopt(long)]
fuel: Option<u64>,

and then when configuring the config and store we would do this:

// When configuring the config...
if self.common.fuel.is_some() {
    config.consume_fuel(true);
}

// ...

// When configuring the store...
if let Some(fuel) = self.common.fuel {
    store.add_fuel(fuel)?;
}

Does that make sense?

[fitzgen]

Suggested change

	#[structopt(long, default_value = "0")]
	add_fuel: u64,
	#[structopt(long, requires = "consume_fuel")]
	add_fuel: Option<u64>,

[fitzgen]

Suggested change

	// if consume fuel has been flagged, we want to instantiate the
	// store with the set amount of fuel from the 'add_fuel' option
	if self.common.consume_fuel {
	store.add_fuel(self.common.add_fuel)?;
	}
	// warn the user if 'add_fuel' is set - is not 0 - and 'consume_fuel' is not
	if self.common.add_fuel != 0 && !self.common.consume_fuel {
	eprintln!(
	"warning: using `--add-fuel` without '--consume-fuel' flag\
	being set. This currently won't do anything."
	);
	}
	if self.common.consume_fuel {
	let fuel = self.common.add_fuel.expect("both add_fuel and consume_fuel must be given");
	store.add_fuel(fuel)?;
	}

[fitzgen]

Suggested change

	#[structopt(long)]
	consume_fuel: bool,
	#[structopt(long, requires = "add_fuel")]
	consume_fuel: bool,

[ilikepi63]

The only concern I could see from --fuel is the point where someone could call without --add-fuel to indicate that they want to --consume-fuel with 0 fuel.

I am not sure if there is a use case, but just thought I would mention it.

[fitzgen]

The only concern I could see from --fuel is the point where someone could call without --add-fuel to indicate that they want to --consume-fuel with 0 fuel.

In that case they would be able to do --fuel 0.

[fitzgen]

Thanks! This looks great, I just have a couple tiny nitpicks on the wording of the comments. In general, we try to use capitalization and punctuation in our code comments.

Once you commit these suggestions, I'll hit the merge button.

Thanks again!

[fitzgen]

Once you commit these suggestions, I'll hit the merge button.

Or I guess github lets me add those suggestions as commits to your branch :)

[ilikepi63]

Thanks @fitzgen!

Learnt a lot from this. Eager to try some more once I get a better understanding of compiler architecture. Thanks again for all the input, you are the best!