Support variadic interface arguments (fixes #8159)

[nox]

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[nox]

unsafe extern fn passVariadicInterface(cx: *mut JSContext, _obj: HandleObject, this: *const TestBinding, args: *const JSJitMethodCallArgs) -> bool {
    let this = &*this;
    let args = &*args;
    let argc = args._base.argc_;
    let mut arg0 = RootedVec::new();
    *arg0 = Vec::with_capacity((argc - 0) as usize);
    for variadicArg in 0..argc {
        let slot: *const Blob = if args.get(variadicArg).get().is_object() {
            match native_from_handlevalue(args.get(variadicArg)) {
                Ok(val) => val,
                Err(()) => {
                    throw_type_error(cx, "value does not implement interface Blob.");
                    return false;
                }
            }
        } else {
            throw_type_error(cx, "Value is not an object.");
            return false;
        };
        arg0.push(JS::from_ref(&*slot));
    }
    let result: () = this.PassVariadicInterface(arg0.r());

    (result).to_jsval(cx, args.rval());
    return true;
}

[nox]

#8159

[Ms2ger]

Not sure I'm a big fan of the branching

[nox]

I have no idea what your remark is about, @Ms2ger.

[jdm]

@bors-servo: r+
This looks ok to me and is necessary for #8225. I'll double back and do a thorough post-merge review.

[bors-servo]

📌 Commit 88bf688 has been approved by jdm

[jdm]

@bors-servo: r-
The pressure's off. I'll review this properly.

[bors-servo]

☔ The latest upstream changes (presumably #8241) made this pull request unmergeable. Please resolve the merge conflicts.

[jdm]

Generally looks good! I'd just like to make sure that we're thinking through the changes to the types carefully.
-S-awaiting-review +S-needs-code-changes

---

Reviewed 10 of 10 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 7 of 7 files at r5.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 761 [r4] (raw file):}
memberType appears to be unused now. We should remove it if that's the case.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1209 [r4] (raw file):}
We lost this check; is that a problem? For example, could that end up causing underflow in the earlier subtraction?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 108 [r5] (raw file):}
"Returns a string of the Rust expression..."

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 764 [r5] (raw file):}
I guess this else block is why we continue rooting members of dictionaries after this change. I'm concerned that this is both non-obvious and confusing.

---

^{components/script/dom/bindings/codegen/Configuration.py, line 166 [r5] (raw file):}
I'm slightly concerned that this could make us no longer root dictionary members or something? What's the effect of modifying this and the next line?

---

^{components/script/dom/bindings/conversions.rs, line 756 [r5] (raw file):}
Let's not make this public if it doesn't need to be. The fewer things expose unsafe pointers the better.

---

Comments from the review on Reviewable.io

[nox]

-S-needs-rebase +S-awaiting-answer

---

Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 761 [r4] (raw file):}
Done.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1209 [r4] (raw file):}
The number of arguments can never be less than the offset where variadic arguments start, when that code is executed. No underflow can thus happen.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 108 [r5] (raw file):}
Done.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 764 [r5] (raw file):}
So what do you want me to do?

---

^{components/script/dom/bindings/codegen/Configuration.py, line 166 [r5] (raw file):}
None apart from letting me distinguish variadic interface arguments from variadic union arguments, etc. Dictionary members stay rooted AFAIK. What codegen result do you want me to paste to convince you?

---

^{components/script/dom/bindings/conversions.rs, line 756 [r5] (raw file):}
Done.

---

Comments from the review on Reviewable.io

[jdm]

-S-awaiting-review -S-awaiting-answer +S-needs-code-changes

---

Reviewed 14 of 14 files at r6.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 764 [r5] (raw file):}
I think my confusion stems from two parts:

• there's no block that explicitly deals with isMember != "Variadic"
• we rely on returnType being Root for things that are not conceptually return types
  I would be happy with an elif for the first point that uses returnType but has a comment explaining our reasoning.

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+

---

Reviewed 1 of 1 files at r7.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit acb13dc has been approved by jdm

[bors-servo]

⌛ Testing commit acb13dc with merge f03a826...

[bors-servo]

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt