Skip to content

Commit

Permalink
Drop support for old-style *.witx files (bytecodealliance#195)
Browse files Browse the repository at this point in the history 
I today started looking at updating this project to the new draft of the
canonical ABI specified at WebAssembly/component-model#23 and while
there's quite a few changes that need to happen the first thing that
struck me was that maintaining all of the support for old-style witx
files is pretty onerous. Especially the ABI-calculating code has lots of
special cases for various witx constructs and the Preview1 ABI, and I
wasn't really sure how to update this in many situations.

Overall the original purpose of the witx support was to prove out that
it's possible to support both the old witx abi and the new canonical ABI
in the same generator. The canonical ABI has changed significantly in
the meantime however and this doesn't necessarily make sense to do any
more. I think it would be best now to reevaluate at the point when WASI
is ready to switch to the component model what to do with the old witx
support. I no longer think that "build it in here" is the obvious
option. As this diff shows there's quite a bit of weight to carry the
old witx abis as this commit clocks in at nearly 7000 lines removed.

The specifics being dropped here are:

* Parsing support for `*.witx`
* Support for `Pointer` and `ConstPointer`
* Support for `WitxInstruction`
* Support for push/pull buffers

The push/pull buffer feature was never actually fully implemented, even
for Rust hosts they only kind-of worked. Most other generators never
even implemented support for them. Additionally support for other
`*.witx` constructs was somewhat spotty at best with very few tests.

My hope is that there are no existing users of this support. If there
are then I think it's best to re-evaluate how best to solve the scenario
on-hand.
  • Loading branch information
@alexcrichton @willemneal
alexcrichton authored and willemneal committed May 31, 2022
1 parent dafadfe commit d42fb31
Show file tree
Hide file tree
Showing 69 changed files with 231 additions and 6,741 deletions.
14 changes: 1 addition & 13 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test = false
[dependencies]
anyhow = "1.0"
structopt = { version = "0.3", default-features = false }
wit-bindgen-gen-core = { path = 'crates/gen-core', features = ['witx-compat'] }
wit-bindgen-gen-core = { path = 'crates/gen-core' }
wit-bindgen-gen-rust-wasm = { path = 'crates/gen-rust-wasm', features = ['structopt'] }
wit-bindgen-gen-wasmtime = { path = 'crates/gen-wasmtime', features = ['structopt'] }
wit-bindgen-gen-wasmtime-py = { path = 'crates/gen-wasmtime-py', features = ['structopt'] }
Expand Down
3 changes: 0 additions & 3 deletions crates/gen-c/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,3 @@ structopt = { version = "0.3", default-features = false, optional = true }

[dev-dependencies]
test-helpers = { path = '../test-helpers', features = ['wit-bindgen-gen-c'] }

[features]
witx-compat = ['wit-bindgen-gen-core/witx-compat']
150 changes: 6 additions & 144 deletions crates/gen-c/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use heck::*;
use std::collections::{BTreeSet, HashMap, HashSet};
use std::mem;
use wit_bindgen_gen_core::wit_parser::abi::{
AbiVariant, Bindgen, Bitcast, Instruction, LiftLower, WasmType, WitxInstruction,
AbiVariant, Bindgen, Bitcast, Instruction, LiftLower, WasmType,
};
use wit_bindgen_gen_core::{wit_parser::*, Direction, Files, Generator, Ns};

Expand Down Expand Up @@ -173,12 +173,7 @@ impl C {
TypeDefKind::Type(t) => self.is_arg_by_pointer(iface, t),
TypeDefKind::Variant(v) => !v.is_enum(),
TypeDefKind::Record(r) if r.is_flags() => false,
TypeDefKind::Pointer(_)
| TypeDefKind::ConstPointer(_)
| TypeDefKind::Record(_)
| TypeDefKind::List(_)
| TypeDefKind::PushBuffer(_)
| TypeDefKind::PullBuffer(_) => true,
TypeDefKind::Record(_) | TypeDefKind::List(_) => true,
},
_ => false,
}
Expand Down Expand Up @@ -257,24 +252,12 @@ impl C {
self.print_ty_name(iface, &Type::Id(*id));
self.src.h("_t");
}
TypeDefKind::Pointer(t) => {
self.print_ty(iface, t);
self.src.h("*");
}
TypeDefKind::ConstPointer(t) => {
self.src.h("const ");
self.print_ty(iface, t);
self.src.h("*");
}
TypeDefKind::List(Type::Char) => {
self.print_namespace(iface);
self.src.h("string_t");
self.needs_string = true;
}
TypeDefKind::Record(_)
| TypeDefKind::List(_)
| TypeDefKind::PushBuffer(_)
| TypeDefKind::PullBuffer(_) => {
TypeDefKind::Record(_) | TypeDefKind::List(_) => {
self.public_anonymous_types.insert(*id);
self.private_anonymous_types.remove(id);
self.print_namespace(iface);
Expand Down Expand Up @@ -339,27 +322,11 @@ impl C {
unimplemented!();
}
}
TypeDefKind::Pointer(t) => {
self.src.h("ptr_");
self.print_ty_name(iface, t);
}
TypeDefKind::ConstPointer(t) => {
self.src.h("const_ptr_ ");
self.print_ty_name(iface, t);
}
TypeDefKind::List(Type::Char) => self.src.h("string"),
TypeDefKind::List(t) => {
self.src.h("list_");
self.print_ty_name(iface, t);
}
TypeDefKind::PushBuffer(t) => {
self.src.h("push_buffer_");
self.print_ty_name(iface, t);
}
TypeDefKind::PullBuffer(t) => {
self.src.h("pull_buffer_");
self.print_ty_name(iface, t);
}
}
}
}
Expand All @@ -370,7 +337,7 @@ impl C {
self.src.h("typedef ");
let kind = &iface.types[ty].kind;
match kind {
TypeDefKind::Type(_) | TypeDefKind::Pointer(_) | TypeDefKind::ConstPointer(_) => {
TypeDefKind::Type(_) => {
unreachable!()
}
TypeDefKind::Record(r) => {
Expand Down Expand Up @@ -424,17 +391,6 @@ impl C {
self.src.h("size_t len;\n");
self.src.h("}");
}
TypeDefKind::PushBuffer(t) | TypeDefKind::PullBuffer(t) => {
self.src.h("struct {\n");
self.src.h("int32_t is_handle;\n");
if let TypeDefKind::PullBuffer(_) = kind {
self.src.h("const ");
}
self.print_ty(iface, t);
self.src.h(" *ptr;\n");
self.src.h("size_t len;\n");
self.src.h("}");
}
}
self.src.h(" ");
self.print_namespace(iface);
Expand Down Expand Up @@ -565,11 +521,6 @@ impl C {
}
self.src.c("}\n");
}

TypeDefKind::PushBuffer(_)
| TypeDefKind::PullBuffer(_)
| TypeDefKind::Pointer(_)
| TypeDefKind::ConstPointer(_) => {}
}
self.src.c("}\n");
}
Expand All @@ -584,8 +535,6 @@ impl C {
TypeDefKind::Type(t) => self.owns_anything(iface, t),
TypeDefKind::Record(r) => r.fields.iter().any(|t| self.owns_anything(iface, &t.ty)),
TypeDefKind::List(_) => true,
TypeDefKind::PushBuffer(_) | TypeDefKind::PullBuffer(_) => false,
TypeDefKind::Pointer(_) | TypeDefKind::ConstPointer(_) => false,
TypeDefKind::Variant(v) => v
.cases
.iter()
Expand Down Expand Up @@ -640,14 +589,7 @@ impl Return {
TypeDefKind::Record(_) => self.splat_tuples(iface, ty, orig_ty),

// other records/lists/buffers always go to return pointers
TypeDefKind::List(_) | TypeDefKind::PushBuffer(_) | TypeDefKind::PullBuffer(_) => {
self.retptrs.push(*orig_ty)
}

// pointers are scalars
TypeDefKind::Pointer(_) | TypeDefKind::ConstPointer(_) => {
self.scalar = Some(Scalar::Type(*orig_ty));
}
TypeDefKind::List(_) => self.retptrs.push(*orig_ty),

// Enums are scalars (this includes bools)
TypeDefKind::Variant(v) if v.is_enum() => {
Expand Down Expand Up @@ -713,7 +655,7 @@ impl Return {
impl Generator for C {
fn preprocess_one(&mut self, iface: &Interface, dir: Direction) {
let variant = Self::abi_variant(dir);
self.sizes.fill(variant, iface);
self.sizes.fill(iface);
self.in_import = variant == AbiVariant::GuestImport;

for func in iface.functions.iter() {
Expand Down Expand Up @@ -879,56 +821,10 @@ impl Generator for C {
.insert(id, mem::replace(&mut self.src.header, prev));
}

fn type_pointer(
&mut self,
iface: &Interface,
_id: TypeId,
name: &str,
const_: bool,
ty: &Type,
docs: &Docs,
) {
drop((iface, _id, name, const_, ty, docs));
}

fn type_builtin(&mut self, iface: &Interface, _id: TypeId, name: &str, ty: &Type, docs: &Docs) {
drop((iface, _id, name, ty, docs));
}

fn type_push_buffer(
&mut self,
iface: &Interface,
id: TypeId,
name: &str,
ty: &Type,
docs: &Docs,
) {
self.type_pull_buffer(iface, id, name, ty, docs);
}

fn type_pull_buffer(
&mut self,
iface: &Interface,
id: TypeId,
name: &str,
ty: &Type,
docs: &Docs,
) {
let prev = mem::take(&mut self.src.header);
self.docs(docs);
self.src.h("typedef struct {\n");
self.src.h("int32_t is_handle;\n");
self.print_ty(iface, ty);
self.src.h(" *ptr;\n");
self.src.h("size_t len;\n");
self.src.h("} ");
self.print_namespace(iface);
self.src.h(&name.to_snake_case());
self.src.h("_t;\n");
self.types
.insert(id, mem::replace(&mut self.src.header, prev));
}

fn import(&mut self, iface: &Interface, func: &Function) {
assert!(!func.is_async, "async not supported yet");
let prev = mem::take(&mut self.src);
Expand Down Expand Up @@ -1365,13 +1261,6 @@ impl Bindgen for FunctionBindgen<'_> {
self.blocks.push((src.into(), mem::take(operands)));
}

fn allocate_typed_space(&mut self, iface: &Interface, ty: TypeId) -> String {
let ty = self.gen.type_string(iface, &Type::Id(ty));
let name = self.locals.tmp("tmp");
self.src.push_str(&format!("{} {};\n", ty, name));
format!("(int32_t) (&{})", name)
}

fn i64_return_pointer_area(&mut self, amt: usize) -> String {
assert!(amt <= self.gen.i64_return_pointer_area_size);
let ptr = self.locals.tmp("ptr");
Expand Down Expand Up @@ -1672,18 +1561,6 @@ impl Bindgen for FunctionBindgen<'_> {
Instruction::IterElem { .. } => results.push("e".to_string()),
Instruction::IterBasePointer => results.push("base".to_string()),

Instruction::BufferLowerPtrLen { .. } => {
drop(self.blocks.pop().unwrap());
results.push(format!("({}).is_handle", operands[0]));
results.push(format!("(int32_t) ({}).ptr", operands[0]));
results.push(format!("({}).len", operands[0]));
}

Instruction::BufferLiftHandle { .. } => {
drop(self.blocks.pop().unwrap());
results.push(format!("({}).idx", operands[0]));
}

Instruction::CallWasm { sig, .. } => {
match sig.results.len() {
0 => {}
Expand Down Expand Up @@ -1897,21 +1774,6 @@ impl Bindgen for FunctionBindgen<'_> {
self.src.push_str("INVALIDSTORE");
}

Instruction::Witx { instr } => match instr {
WitxInstruction::I32FromPointer | WitxInstruction::I32FromConstPointer => {
results.push(format!("(int32_t) ({})", operands[0]))
}
WitxInstruction::AddrOf => {
results.push(format!("(int32_t) (&{})", operands[0]));
}
WitxInstruction::ReuseReturn => {
results.push(self.wasm_return.clone().unwrap());
}
i => unimplemented!("{:?}", i),
},

Instruction::BufferPayloadName => results.push("INVALID".into()),

i => unimplemented!("{:?}", i),
}
}
Expand Down
3 changes: 0 additions & 3 deletions crates/gen-core/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,3 @@ doctest = false
[dependencies]
wit-parser = { path = '../parser' }
anyhow = "1"

[features]
witx-compat = ['wit-parser/witx-compat']
Loading

0 comments on commit d42fb31

Please sign in to comment.