Skip to content

Commit 1f9e602

Browse files
authored
Drop support for old-style *.witx files (#195)
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.
1 parent bb33644 commit 1f9e602

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+231
-6741
lines changed

Cargo.lock

Lines changed: 1 addition & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ test = false
2121
[dependencies]
2222
anyhow = "1.0"
2323
structopt = { version = "0.3", default-features = false }
24-
wit-bindgen-gen-core = { path = 'crates/gen-core', features = ['witx-compat'] }
24+
wit-bindgen-gen-core = { path = 'crates/gen-core' }
2525
wit-bindgen-gen-rust-wasm = { path = 'crates/gen-rust-wasm', features = ['structopt'] }
2626
wit-bindgen-gen-wasmtime = { path = 'crates/gen-wasmtime', features = ['structopt'] }
2727
wit-bindgen-gen-wasmtime-py = { path = 'crates/gen-wasmtime-py', features = ['structopt'] }

crates/gen-c/Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,3 @@ structopt = { version = "0.3", default-features = false, optional = true }
1515

1616
[dev-dependencies]
1717
test-helpers = { path = '../test-helpers', features = ['wit-bindgen-gen-c'] }
18-
19-
[features]
20-
witx-compat = ['wit-bindgen-gen-core/witx-compat']

crates/gen-c/src/lib.rs

Lines changed: 6 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use heck::*;
22
use std::collections::{BTreeSet, HashMap, HashSet};
33
use std::mem;
44
use wit_bindgen_gen_core::wit_parser::abi::{
5-
AbiVariant, Bindgen, Bitcast, Instruction, LiftLower, WasmType, WitxInstruction,
5+
AbiVariant, Bindgen, Bitcast, Instruction, LiftLower, WasmType,
66
};
77
use wit_bindgen_gen_core::{wit_parser::*, Direction, Files, Generator, Ns};
88

@@ -173,12 +173,7 @@ impl C {
173173
TypeDefKind::Type(t) => self.is_arg_by_pointer(iface, t),
174174
TypeDefKind::Variant(v) => !v.is_enum(),
175175
TypeDefKind::Record(r) if r.is_flags() => false,
176-
TypeDefKind::Pointer(_)
177-
| TypeDefKind::ConstPointer(_)
178-
| TypeDefKind::Record(_)
179-
| TypeDefKind::List(_)
180-
| TypeDefKind::PushBuffer(_)
181-
| TypeDefKind::PullBuffer(_) => true,
176+
TypeDefKind::Record(_) | TypeDefKind::List(_) => true,
182177
},
183178
_ => false,
184179
}
@@ -257,24 +252,12 @@ impl C {
257252
self.print_ty_name(iface, &Type::Id(*id));
258253
self.src.h("_t");
259254
}
260-
TypeDefKind::Pointer(t) => {
261-
self.print_ty(iface, t);
262-
self.src.h("*");
263-
}
264-
TypeDefKind::ConstPointer(t) => {
265-
self.src.h("const ");
266-
self.print_ty(iface, t);
267-
self.src.h("*");
268-
}
269255
TypeDefKind::List(Type::Char) => {
270256
self.print_namespace(iface);
271257
self.src.h("string_t");
272258
self.needs_string = true;
273259
}
274-
TypeDefKind::Record(_)
275-
| TypeDefKind::List(_)
276-
| TypeDefKind::PushBuffer(_)
277-
| TypeDefKind::PullBuffer(_) => {
260+
TypeDefKind::Record(_) | TypeDefKind::List(_) => {
278261
self.public_anonymous_types.insert(*id);
279262
self.private_anonymous_types.remove(id);
280263
self.print_namespace(iface);
@@ -339,27 +322,11 @@ impl C {
339322
unimplemented!();
340323
}
341324
}
342-
TypeDefKind::Pointer(t) => {
343-
self.src.h("ptr_");
344-
self.print_ty_name(iface, t);
345-
}
346-
TypeDefKind::ConstPointer(t) => {
347-
self.src.h("const_ptr_ ");
348-
self.print_ty_name(iface, t);
349-
}
350325
TypeDefKind::List(Type::Char) => self.src.h("string"),
351326
TypeDefKind::List(t) => {
352327
self.src.h("list_");
353328
self.print_ty_name(iface, t);
354329
}
355-
TypeDefKind::PushBuffer(t) => {
356-
self.src.h("push_buffer_");
357-
self.print_ty_name(iface, t);
358-
}
359-
TypeDefKind::PullBuffer(t) => {
360-
self.src.h("pull_buffer_");
361-
self.print_ty_name(iface, t);
362-
}
363330
}
364331
}
365332
}
@@ -370,7 +337,7 @@ impl C {
370337
self.src.h("typedef ");
371338
let kind = &iface.types[ty].kind;
372339
match kind {
373-
TypeDefKind::Type(_) | TypeDefKind::Pointer(_) | TypeDefKind::ConstPointer(_) => {
340+
TypeDefKind::Type(_) => {
374341
unreachable!()
375342
}
376343
TypeDefKind::Record(r) => {
@@ -424,17 +391,6 @@ impl C {
424391
self.src.h("size_t len;\n");
425392
self.src.h("}");
426393
}
427-
TypeDefKind::PushBuffer(t) | TypeDefKind::PullBuffer(t) => {
428-
self.src.h("struct {\n");
429-
self.src.h("int32_t is_handle;\n");
430-
if let TypeDefKind::PullBuffer(_) = kind {
431-
self.src.h("const ");
432-
}
433-
self.print_ty(iface, t);
434-
self.src.h(" *ptr;\n");
435-
self.src.h("size_t len;\n");
436-
self.src.h("}");
437-
}
438394
}
439395
self.src.h(" ");
440396
self.print_namespace(iface);
@@ -565,11 +521,6 @@ impl C {
565521
}
566522
self.src.c("}\n");
567523
}
568-
569-
TypeDefKind::PushBuffer(_)
570-
| TypeDefKind::PullBuffer(_)
571-
| TypeDefKind::Pointer(_)
572-
| TypeDefKind::ConstPointer(_) => {}
573524
}
574525
self.src.c("}\n");
575526
}
@@ -584,8 +535,6 @@ impl C {
584535
TypeDefKind::Type(t) => self.owns_anything(iface, t),
585536
TypeDefKind::Record(r) => r.fields.iter().any(|t| self.owns_anything(iface, &t.ty)),
586537
TypeDefKind::List(_) => true,
587-
TypeDefKind::PushBuffer(_) | TypeDefKind::PullBuffer(_) => false,
588-
TypeDefKind::Pointer(_) | TypeDefKind::ConstPointer(_) => false,
589538
TypeDefKind::Variant(v) => v
590539
.cases
591540
.iter()
@@ -640,14 +589,7 @@ impl Return {
640589
TypeDefKind::Record(_) => self.splat_tuples(iface, ty, orig_ty),
641590

642591
// other records/lists/buffers always go to return pointers
643-
TypeDefKind::List(_) | TypeDefKind::PushBuffer(_) | TypeDefKind::PullBuffer(_) => {
644-
self.retptrs.push(*orig_ty)
645-
}
646-
647-
// pointers are scalars
648-
TypeDefKind::Pointer(_) | TypeDefKind::ConstPointer(_) => {
649-
self.scalar = Some(Scalar::Type(*orig_ty));
650-
}
592+
TypeDefKind::List(_) => self.retptrs.push(*orig_ty),
651593

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

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

882-
fn type_pointer(
883-
&mut self,
884-
iface: &Interface,
885-
_id: TypeId,
886-
name: &str,
887-
const_: bool,
888-
ty: &Type,
889-
docs: &Docs,
890-
) {
891-
drop((iface, _id, name, const_, ty, docs));
892-
}
893-
894824
fn type_builtin(&mut self, iface: &Interface, _id: TypeId, name: &str, ty: &Type, docs: &Docs) {
895825
drop((iface, _id, name, ty, docs));
896826
}
897827

898-
fn type_push_buffer(
899-
&mut self,
900-
iface: &Interface,
901-
id: TypeId,
902-
name: &str,
903-
ty: &Type,
904-
docs: &Docs,
905-
) {
906-
self.type_pull_buffer(iface, id, name, ty, docs);
907-
}
908-
909-
fn type_pull_buffer(
910-
&mut self,
911-
iface: &Interface,
912-
id: TypeId,
913-
name: &str,
914-
ty: &Type,
915-
docs: &Docs,
916-
) {
917-
let prev = mem::take(&mut self.src.header);
918-
self.docs(docs);
919-
self.src.h("typedef struct {\n");
920-
self.src.h("int32_t is_handle;\n");
921-
self.print_ty(iface, ty);
922-
self.src.h(" *ptr;\n");
923-
self.src.h("size_t len;\n");
924-
self.src.h("} ");
925-
self.print_namespace(iface);
926-
self.src.h(&name.to_snake_case());
927-
self.src.h("_t;\n");
928-
self.types
929-
.insert(id, mem::replace(&mut self.src.header, prev));
930-
}
931-
932828
fn import(&mut self, iface: &Interface, func: &Function) {
933829
assert!(!func.is_async, "async not supported yet");
934830
let prev = mem::take(&mut self.src);
@@ -1365,13 +1261,6 @@ impl Bindgen for FunctionBindgen<'_> {
13651261
self.blocks.push((src.into(), mem::take(operands)));
13661262
}
13671263

1368-
fn allocate_typed_space(&mut self, iface: &Interface, ty: TypeId) -> String {
1369-
let ty = self.gen.type_string(iface, &Type::Id(ty));
1370-
let name = self.locals.tmp("tmp");
1371-
self.src.push_str(&format!("{} {};\n", ty, name));
1372-
format!("(int32_t) (&{})", name)
1373-
}
1374-
13751264
fn i64_return_pointer_area(&mut self, amt: usize) -> String {
13761265
assert!(amt <= self.gen.i64_return_pointer_area_size);
13771266
let ptr = self.locals.tmp("ptr");
@@ -1672,18 +1561,6 @@ impl Bindgen for FunctionBindgen<'_> {
16721561
Instruction::IterElem { .. } => results.push("e".to_string()),
16731562
Instruction::IterBasePointer => results.push("base".to_string()),
16741563

1675-
Instruction::BufferLowerPtrLen { .. } => {
1676-
drop(self.blocks.pop().unwrap());
1677-
results.push(format!("({}).is_handle", operands[0]));
1678-
results.push(format!("(int32_t) ({}).ptr", operands[0]));
1679-
results.push(format!("({}).len", operands[0]));
1680-
}
1681-
1682-
Instruction::BufferLiftHandle { .. } => {
1683-
drop(self.blocks.pop().unwrap());
1684-
results.push(format!("({}).idx", operands[0]));
1685-
}
1686-
16871564
Instruction::CallWasm { sig, .. } => {
16881565
match sig.results.len() {
16891566
0 => {}
@@ -1897,21 +1774,6 @@ impl Bindgen for FunctionBindgen<'_> {
18971774
self.src.push_str("INVALIDSTORE");
18981775
}
18991776

1900-
Instruction::Witx { instr } => match instr {
1901-
WitxInstruction::I32FromPointer | WitxInstruction::I32FromConstPointer => {
1902-
results.push(format!("(int32_t) ({})", operands[0]))
1903-
}
1904-
WitxInstruction::AddrOf => {
1905-
results.push(format!("(int32_t) (&{})", operands[0]));
1906-
}
1907-
WitxInstruction::ReuseReturn => {
1908-
results.push(self.wasm_return.clone().unwrap());
1909-
}
1910-
i => unimplemented!("{:?}", i),
1911-
},
1912-
1913-
Instruction::BufferPayloadName => results.push("INVALID".into()),
1914-
19151777
i => unimplemented!("{:?}", i),
19161778
}
19171779
}

crates/gen-core/Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,3 @@ doctest = false
1010
[dependencies]
1111
wit-parser = { path = '../parser' }
1212
anyhow = "1"
13-
14-
[features]
15-
witx-compat = ['wit-parser/witx-compat']

0 commit comments

Comments
 (0)