webidl: Add a method of whitelisting an API as not mutating its byte slice argument

[alexcrichton]

Spawned off from #978, we currently pessmize all slices to be mutable when passed to JS. This conservative decision is required for correctness, but we typically know on a one-off basis that the mutable borrow isn't actually necessary.

We should add a specific method to whitelist particular APIs and switch the to using an immutable slice instead of a mutable one!

[RReverser]

I've just found that TextDecoder::decode_with_u8_array{_and_options} requires a mutable slice. Sounds relevant to this issue? I'll be happy to send a PR, but what do I need to do to whitelist these methods?

[alexcrichton]

@RReverser just to confirm, but do you mean it doesn't need a mutable slice? (it looks like it already takes a mutable slice)

[RReverser]

Yeah, I'm saying that I found that it currently requires mutable slice in the Rust API, but it shouldn't.

[alexcrichton]

Ah yes, then indeed this is the right issue! Currently a whitelist isn't implemented, though, that would need to be done first. (that's what this issue is about)

[RReverser]

Ah sorry, I misread, I thought the issue was merely about finding and marking methods that need to accept immutable slices.

[chinedufn]

@alexcrichton where might one start looking in order to implement the whitelist? Any tips / guidance for diving in here?

---

Background:

You end up running into minor inconveniences very frequently when working on WebGL applications since you're using a bunch of these APIs that require mutable slices very frequently.

Nothing too, too painful - just inconvenient.

---

So if this is do-able for a beginner contributor I'd love to take a look!

Thanks!

[alexcrichton]

Sure yeah! Here's at least one way I think this could be implemented today:

• First up, probably some cleanup needs to be done to enum IdlType because everything becomes mutable slices. For example Uint8ArrayMut probably isn't needed any more as opposed to Uint8Array
• Next, We'll want to add a variant that represents immutable slices. Perhaps booleans on the existing arrays, or perhaps new variants. After an enum variant is added appropriate places are fixed in compilation to ensure shared slices instead of mutable slices are used in Rust.
• Finally, we'll want to actually generate these new types when generating bindings. More on that now...

Digging around again, the main point of creation of the mutable type vs immutable happens here. I think we'll want to change that to something like:

let idl_type = arg.ty.to_idl_type(self);
let idl_type = maybe_adjust(idl_type, id);
idl_args.push(idl_type);

The maybe_adjust method would implement a whitelist that would take a look at the id, and if it's OperationId::Operation(Some(s)) then it'll check s against a whitelist and convert any mutable slice into an immutable slice. (or something like that)

And I think that may be good enough to start here?

[fitzgen]

One thing that is not clear to me is how we intend to deal with semver here -- changing an &mut [u8] to a &[u8] is a breaking change, yeah? I'd prefer not to require a breaking release for web-sys...

[alexcrichton]

I'm personally hoping that because &mut [u8] coerces to &[u8] we'll be able to get by without breaking changes, but if that doesn't end up being the case then we'll definitely want to batch these up and do a more thorough review

[chinedufn]

Super helpful guidance thanks! Should be taking a look later this week.

[raphlinus]

Came here to point out that ImageData.html::new_with_u8_clamped_array has this issue, and I'm currently doing an extra clone in piet-web to work around it. Hopefully if you do a thorough review, you'll catch that case, but if you're doing them one-off, please add to the list.

[alexcrichton]

@fitzgen FWIW in terms of literal code, I think this is a non-breaking change, this compiles on the playground:

fn a(_: &[u8]) {}


fn b() {
    a(&mut Vec::new());
    a(&mut []);
    a(&mut [1]);
    a(&mut [1;2]);
    a(&mut [3][..]);
}

If, however, someone did something like let x: fn(&mut [u8]) = ... then that could be a problem, but I think we can probably land whitelisting without worrying too much about that.

[fitzgen]

SGTM 👍

[alexcrichton]

Just hasn't been done yet, PRs are always welcome to update the whitelist!