First steps towards T:Read parsing (BrTable)

[Robbepop]

Moved from: bytecodealliance/wasmparser#216

First step towards implementing https://github.com/bytecodealliance/wasmparser/issues/90.

This refactoring of BrTable is required in order to drop the explicit lifetime from the Operator enum. We introduce the WasmBrTable and WasmBrTableBuilder traits so that we can later make Operator generic over user provided types for BrTable and maybe other custom Wasm operators and types in the future.

We separated BrTable and BrTableBuilder in order to make the more important BrTable less constraint regarding incremental building. At the moment we do not provide users with a way to customize the Operator enum so we might experience slight performance regressions since a BrTable now is eagerly constructed instead of deferring its construction.

In the long run this procedure should even improve performance since we no longer need to parse the br_table targets twice. Also in some cases we needed to construct an internal br_table and users would then construct their own custom br_table on their side. With this (and follow-up) changes we simply directly construct the user defined br_table directly.

Next Steps

After merging of this PR the next steps to make the parser work on T: Read instead of working on a slice of [u8] are:

• Maybe we want to allow constructing Result<Self::BrTable> in WasmBrTableBuilder trait to allow for use cases where a user doesn't want to support br_table at all.
• Find any other locations where a lifetime is used in some produced data structure as it was the case for Operator and replace it with similar technique.
• Introduce collective trait that can be used to pass to the parser to tell it which user defined types to construct. The user has to implement this trait for some super type to enumerate all the user defined types. So far this is only needed for BrTable but this design is scalable for future additions.
• Change API to use T: Read.

Benchmarks

Sadly I cannot test the performance implications since the benchmark harness seems to be broken on master. At least I get the following output on cargo bench with RUST_BACKTRACE=1:

Benchmarking it works benchmark: Warming up for 3.0000 sthread 'main' panicked at 'unexpected error Bad magic number (at offset 0)', benches/benchmark.rs:38:42
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1069
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
  11: rust_begin_unwind
             at src/libstd/panicking.rs:385
  12: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:339
  13: criterion::Bencher<M>::iter
  14: <criterion::routine::Function<M,F,T> as criterion::routine::Routine<M,T>>::warm_up
  15: criterion::routine::Routine::sample
  16: criterion::analysis::common
  17: criterion::benchmark_group::BenchmarkGroup<M>::bench_function
  18: criterion::Criterion<M>::bench_function
  19: benchmark::main
  20: std::rt::lang_start::{{closure}}
  21: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  22: std::panicking::try::do_call
             at src/libstd/panicking.rs:297
  23: std::panicking::try
             at src/libstd/panicking.rs:274
  24: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  25: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  26: main
  27: __libc_start_main
  28: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: bench failed

Doc Screenshots

[Robbepop]

Tests in other wasm-tools crates are failing but I do not understand why exactly and how to fix them according to your standards.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', tests/roundtrip.rs:103:18
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:486
  11: rust_begin_unwind
             at src/libstd/panicking.rs:388
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:101
  13: core::option::expect_none_failed
             at src/libcore/option.rs:1272
  14: core::result::Result<T,E>::unwrap
             at /home/robrob/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/result.rs:1005
  15: roundtrip::find_tests::find_tests
             at tests/roundtrip.rs:103
  16: roundtrip::find_tests
             at tests/roundtrip.rs:89
  17: roundtrip::main
             at tests/roundtrip.rs:38
  18: std::rt::lang_start::{{closure}}
             at /home/robrob/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67
  19: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  20: std::panicking::try::do_call
             at src/libstd/panicking.rs:297
  21: std::panicking::try
             at src/libstd/panicking.rs:274
  22: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  23: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  24: std::rt::lang_start
             at /home/robrob/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67
  25: main
  26: __libc_start_main
  27: _start

Running the compare-master.sh script is also not working because some unrelated things are not compiling:

error[E0063]: missing field `enable_tail_call` in initializer of `wasmparser::OperatorValidatorConfig`
 --> crates/wasmparser/benches/benchmark.rs:9:22
  |
9 |     operator_config: OperatorValidatorConfig {
  |                      ^^^^^^^^^^^^^^^^^^^^^^^ missing `enable_tail_call`

error: aborting due to previous error

[Robbepop]

@yurydelendik I'd like to experiment with some already established core functionality of some other parser crates, namely the ParseBuffer. I think if we build the parsing infrastructure of the wasmparser crate upon a T: ParseBuffer foundation we could in theory define the ParseBuffer trait in a way that allows for T: ParseBuffer to exist that do not allocate any heap memory, thus not requiring alloc. Also depending on how we define the ParseBuffer trait we could also eventually make it possible to have the old "lazy" br_table back.

If you like this base idea, I'd love to pioneer into this direction for the wasmparser crate.

[Robbepop]

I guess we can close this since T:Read support has been introduced with another PR already.