C: handling of `@deprecated` items results in breaking changes

[yoshuawuyts]

In https://github.com/WebAssembly/wasi-http/issues/133 @brendanburns reported that upgrading from wasi/[email protected] to wasi/[email protected] resulted in breaking changes in wit-bindgen's C output, causing the following error:

/usr/local/lib/wasi-sdk-22.0/bin/clang -c wasi_http.c -o wasi_http.o
wasi_http.c:48:5: error: unknown type name 'client_tuple2_field_key_field_value_t'; did you mean 'client_tuple2_field_name_field_value_t'?
   48 |     client_tuple2_field_key_field_value_t *headers = malloc(sizeof(client_tuple2_field_key_field_value_t) * wasi_response.headers.len);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     client_tuple2_field_name_field_value_t
./client.h:418:3: note: 'client_tuple2_field_name_field_value_t' declared here
  418 | } client_tuple2_field_name_field_value_t;
      |   ^
wasi_http.c:48:68: error: use of undeclared identifier 'client_tuple2_field_key_field_value_t'
   48 |     client_tuple2_field_key_field_value_t *headers = malloc(sizeof(client_tuple2_field_key_field_value_t) * wasi_response.headers.len);
      |                                                                    ^
wasi_http.c:56:5: error: unknown type name 'client_list_tuple2_field_key_field_value_t'; did you mean 'client_list_tuple2_field_name_field_value_t'?
   56 |     client_list_tuple2_field_key_field_value_t header_list = {
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     client_list_tuple2_field_name_field_value_t
./client.h:423:3: note: 'client_list_tuple2_field_name_field_value_t' declared here
  423 | } client_list_tuple2_field_name_field_value_t;
      |   ^
wasi_http.c:106:5: error: unknown type name 'client_tuple2_field_key_field_value_t'; did you mean 'client_tuple2_field_name_field_value_t'?
  106 |     client_tuple2_field_key_field_value_t content_type[] = {{
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     client_tuple2_field_name_field_value_t
./client.h:418:3: note: 'client_tuple2_field_name_field_value_t' declared here
  418 | } client_tuple2_field_name_field_value_t;
      |   ^
wasi_http.c:114:5: error: unknown type name 'client_list_tuple2_field_key_field_value_t'; did you mean 'client_list_tuple2_field_name_field_value_t'?
  114 |     client_list_tuple2_field_key_field_value_t headers_list = {
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     client_list_tuple2_field_name_field_value_t
./client.h:423:3: note: 'client_list_tuple2_field_name_field_value_t' declared here
  423 | } client_list_tuple2_field_name_field_value_t;
      |   ^
wasi_http.c:194:5: error: unknown type name 'client_list_tuple2_field_key_field_value_t'; did you mean 'client_list_tuple2_field_name_field_value_t'?
  194 |     client_list_tuple2_field_key_field_value_t header_list;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     client_list_tuple2_field_name_field_value_t
./client.h:423:3: note: 'client_list_tuple2_field_name_field_value_t' declared here
  423 | } client_list_tuple2_field_name_field_value_t;
      |   ^
6 errors generated.
make: *** [Makefile:17: wasi_http.o] Error 1

This seems to point at an issue handling @deprecated in WIT. Specifically https://github.com/WebAssembly/wasi-http/pull/127 which marked field-key as deprecated:

+ @deprecated(version = 0.2.2)
  type field-key = string;

The intent of the @deprecated tag is to enable both tooling and host runtimes to alert users an API should no longer be used. But crucially: the API is still considered a required part of the WIT document and is part of the public contract. @deprecated types not being generated by wit-bindgen targeting C strikes me as a bug we should fix.

[jsturtevant]

@brendanburns what version of wit-bindgen are you using? What is the command? Are you using the --rename options?

I am not seeing client_tuple2_field_key_field_value_t in the output of wit-bindgen for c. When using wasi-http 0.2.1. I do see a imports_tuple2_field_key_field_value_t which doesn't appear to be there in wasi-http 0.2.2.

[jsturtevant]

Looking closer at the change in https://github.com/WebAssembly/wasi-http/pull/127/files#diff-a8f42ac7cc164b46639a30d22f97ca796bd47991da55edc0022578930424be06

It seems the tuple that was defined as _tuple2_field_key_field_value_t is no longer in the wit:

    @since(version = 0.2.0)
-   entries: func() -> list<tuple<field-key,field-value>>;
+  entries: func() -> list<tuple<field-name,field-value>>;

This is a super subtle change to the wit that the binding generator no longer needs that tuple type

[jsturtevant]

by changing that function signature, at least the way the c bindings generated works it was a breaking change. In C# the tuple is only typed as a string and so wouldn't change.

I don't know enough about the c binding generator to know if tuples need to be typed strongly like that...

[yoshuawuyts]

Thank you for tracking this down further @jsturtevant!

[alexcrichton]

The change @jsturtevant highlighted makes sense to me and I agree that's probably what caused the breakage here. In general without actual tuple types in C we're left to do the "next best thing". Currently it's a correct assessment that non-breaking changes in WIT can result in breaking changes in the bindings for C, and I'm not sure what to do about that.