mozilla / uniffi-rs

a multi-language bindings generator for rust
https://mozilla.github.io/uniffi-rs/
Mozilla Public License 2.0
2.83k stars 231 forks source link

Cannot use Rust trait exports in UDL without library mode in Kotlin or Swift #1883

Open paxbun opened 11 months ago

paxbun commented 11 months ago

Without library mode, UniFFI 0.25.2 pushes the Rust trait information defined in UDL twice to ComponentInterface, once in ComponentInterface::from_metadata, and once in macro_metadata::add_to_ci_from_library. This makes duplicate overloads of methods like *_uniffi_trait_eq_eq or *_uniffi_trait_debug in generate bindings. This is trivial when using Python, but it makes compilation errors when using Kotlin or Swift.

To reproduce this issue, I modified fixtures/trait-methods/tests/test_generated_bindings.rs and uniffi_bindgen/src/bindings/python/test.rs as follows:

// fixtures/trait-methods/tests/test_generated_bindings.rs
#[test]
fn uniffi_foreign_language_testcase_test_py() -> uniffi::deps::anyhow::Result<()> {
    // use a directory that can be easily opened from the editor or the system explorer
    let tmp_dir = concat!(std::env!("CARGO_MANIFEST_DIR"), "/tests/tmp");
    std::fs::create_dir_all(tmp_dir).unwrap();
    uniffi::python_run_test(
        // std::env!("CARGO_TARGET_TMPDIR"),
        tmp_dir,
        std::env!("CARGO_PKG_NAME"),
        concat!(std::env!("CARGO_MANIFEST_DIR"), "/tests/bindings/test.py"),
    )
}
// uniffi_bindgen/src/bindings/python/test.rs
...
    // generate_bindings(
    //     &cdylib_path,
    //     None,
    //     &[TargetLanguage::Python],
    //     None,
    //     &out_dir,
    //     false,
    // )?;
    crate::generate_bindings(
        Utf8Path::new(concat!(
            std::env!("CARGO_MANIFEST_DIR"),
            "/../fixtures/trait-methods/src/trait_methods.udl"
        )),
        None,
        vec![TargetLanguage::Python],
        Some(&out_dir),
        Some(&cdylib_path),
        None,
        false,
    )?;
...

You can see in fixtures/trait-methods/tests/tmp/uniffi-fixture-trait-methods-{random string}/trait_methods.py, the trait methods are duplicated as follows. Note that even without library mode, Rust trait exports using procedural macros are not duplicated.

# In line 530
_UniffiLib.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display.argtypes = (
    ctypes.c_void_p,
    ctypes.POINTER(_UniffiRustCallStatus),
)
_UniffiLib.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display.restype = _UniffiRustBuffer

# In line 557
_UniffiLib.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display.argtypes = (
    ctypes.c_void_p,
    ctypes.POINTER(_UniffiRustCallStatus),
)
_UniffiLib.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display.restype = _UniffiRustBuffer

# In line 1083
    def __str__(self, ) -> "str":
        return _UniffiConverterString.lift(
            _rust_call(_UniffiLib.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display,self._pointer,)
        )

# In line 1128
    def __str__(self, ) -> "str":
        return _UniffiConverterString.lift(
            _rust_call(_UniffiLib.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display,self._pointer,)
        )

I was implementing Rust trait exports for uniffi-kotlin-multiplatform-bindings but encountered this issue. Please see https://gitlab.com/trixnity/uniffi-kotlin-multiplatform-bindings/-/issues/28 for more details. We're bypassing this issue by using library mode in our unit tests, but I think it needs to be fixed in the original UniFFI repo.

paxbun commented 11 months ago

This is the test result with Kotlin after the same modification: trait_methods.kt

tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:390:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_debug(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_debug(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_debug(`ptr`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:392:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(`ptr`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:394:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(`ptr`: Pointer,`other`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:396:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_ne(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_ne(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_ne(`ptr`: Pointer,`other`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:398:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(`ptr`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:400:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_debug(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_debug(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_debug(`ptr`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:402:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(`ptr`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:404:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(`ptr`: Pointer,`other`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:406:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_ne(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_ne(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_ne(`ptr`: Pointer,`other`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:408:5: error: conflicting overloads: public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib, public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib
    fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(`ptr`: Pointer,_uniffi_out_err: RustCallStatus, 
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:998:5: error: conflicting overloads: public open fun toString(): String defined in uniffi.trait_methods.TraitMethods, public open fun toString(): String defined in uniffi.trait_methods.TraitMethods
    override fun toString(): String =
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1001:25: error: overload resolution ambiguity: 
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
    _UniFFILib.INSTANCE.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(it,
                        ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1006:37: error: type mismatch: inferred type is Unit but RustBuffer.ByValue was expected
            FfiConverterString.lift(it)
                                    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1009:5: error: conflicting overloads: public open fun equals(other: Any?): Boolean defined in uniffi.trait_methods.TraitMethods, public open fun equals(other: Any?): Boolean defined in uniffi.trait_methods.TraitMethods
    override fun equals(other: Any?): Boolean {
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1014:25: error: overload resolution ambiguity: 
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
    _UniFFILib.INSTANCE.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(it,
                        ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1019:38: error: type mismatch: inferred type is Unit but Byte was expected
            FfiConverterBoolean.lift(it)
                                     ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1022:5: error: conflicting overloads: public open fun hashCode(): Int defined in uniffi.trait_methods.TraitMethods, public open fun hashCode(): Int defined in uniffi.trait_methods.TraitMethods
    override fun hashCode(): Int =
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1025:25: error: overload resolution ambiguity: 
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib
    _UniFFILib.INSTANCE.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(it,
                        ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1030:36: error: type mismatch: inferred type is Unit but Long was expected
            FfiConverterULong.lift(it).toInt()
                                   ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1032:5: error: conflicting overloads: public open fun toString(): String defined in uniffi.trait_methods.TraitMethods, public open fun toString(): String defined in uniffi.trait_methods.TraitMethods
    override fun toString(): String =
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1035:25: error: overload resolution ambiguity: 
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(ptr: Pointer, _uniffi_out_err: RustCallStatus): RustBuffer.ByValue defined in uniffi.trait_methods._UniFFILib
    _UniFFILib.INSTANCE.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_display(it,
                        ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1040:37: error: type mismatch: inferred type is Unit but RustBuffer.ByValue was expected
            FfiConverterString.lift(it)
                                    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1043:5: error: conflicting overloads: public open fun equals(other: Any?): Boolean defined in uniffi.trait_methods.TraitMethods, public open fun equals(other: Any?): Boolean defined in uniffi.trait_methods.TraitMethods
    override fun equals(other: Any?): Boolean {
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1048:25: error: overload resolution ambiguity: 
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(ptr: Pointer, other: Pointer, _uniffi_out_err: RustCallStatus): Byte defined in uniffi.trait_methods._UniFFILib
    _UniFFILib.INSTANCE.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_eq_eq(it,
                        ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1053:38: error: type mismatch: inferred type is Unit but Byte was expected
            FfiConverterBoolean.lift(it)
                                     ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1056:5: error: conflicting overloads: public open fun hashCode(): Int defined in uniffi.trait_methods.TraitMethods, public open fun hashCode(): Int defined in uniffi.trait_methods.TraitMethods
    override fun hashCode(): Int =
    ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1059:25: error: overload resolution ambiguity: 
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib
public abstract fun uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(ptr: Pointer, _uniffi_out_err: RustCallStatus): Long defined in uniffi.trait_methods._UniFFILib
    _UniFFILib.INSTANCE.uniffi_uniffi_trait_methods_fn_method_traitmethods_uniffi_trait_hash(it,
                        ^
tests/tmp/uniffi-fixture-trait-methods-9ea5217c19ee0f4a/uniffi/trait_methods/trait_methods.kt:1064:36: error: type mismatch: inferred type is Unit but Long was expected
            FfiConverterULong.lift(it).toInt()
                                   ^
Error: running `kotlinc` failed
test uniffi_foreign_language_testcase_test_kts ... FAILED

failures:

failures:
    uniffi_foreign_language_testcase_test_kts
mhammond commented 11 months ago

We'd probably take a fix for this, but the direction we are heading is to only support library mode, as that's the only way "external types" might work.

ptitjes commented 11 months ago

We'd probably take a fix for this, but the direction we are heading is to only support library mode, as that's the only way "external types" might work.

Do you mean that there would be only one workflow for the user? That is:

  1. use the scaffolding generator, that reads the UDL file and generates a library with metadata included, and then
  2. use the foreign language binding generator that reads the metadata from the library.

Did I understand correctly? If yes do you know when the other workflow would be dropped? (Asking this to plan ahead for our Kotlin Multiplatform bindings and its Gradle plugin...)

mhammond commented 11 months ago

Yeah, that is what I meant - although part of (1) is already optional because UDL files are optional. (2) is necessary so we can "resolve" external types used by the crate or types exposed via procmacros (note that loading the UDL file when generating foreign bindings still isn't going to help you learn about types exposed via procmacros)

I'm not sure we have decided to actually drop support for the other workflow though, especially while it continues to work OK for users who aren't trying to use external types or procmacros - which is why I was suggesting we would take a patch. IIUC, library mode is used exclusively inside both the uniffi-rs repo and inside all other mozilla repos which use uniffi.

ptitjes commented 11 months ago

We'd probably take a fix for this, [...]

Ah yeah, I had read this ^^^^ too quickly...

Thanks for your detailed answer. So I'll try to find from where the duplication comes from and make a PR for this.

ptitjes commented 11 months ago

@mhammond uniffi_macros::build_foreign_language_testcases (which is used in the test fixtures) always use uniffi_bindgen::library_mode::generate_bindings. What would you prefer in order to test a fix to this issue that would use uniffi_bindgen::generate_external_bindings?

The best option I see for now is to add a build_foreign_language_testcases_from_udl function that would pass true to a new from_udl: bool parameter down to the run_script function calls. We would then selectively apply build_foreign_language_testcases_from_udl in addition to current build_foreign_language_testcases. Here I would only apply it to the trait-methods fixture.

What do you think?

mhammond commented 11 months ago

I assume the fix will be such that there's probably no real advantage/requirement to have tests in each of the bindings - indeed, it might even be such that a pure rust test is all we need. But assuming we want just one binding, I'd say we go with Python and hack together a solution for just that 1 crate. If that turns out to be too difficult etc, then what you suggest sounds fine.

ptitjes commented 11 months ago

@mhammond In fact, looking at the code, I don't clearly see what the fix would be...

I mean, in uniffi_bingen::generate_external_bindings we have those four consecutive lines:

    let mut component = parse_udl(udl_file.as_ref(), &crate_name)?;
    if let Some(ref library_file) = library_file {
        macro_metadata::add_to_ci_from_library(&mut component, library_file.as_ref())?;
    }

The first one gets metadata by parsing the UDL and the other three lines get metadata from the library.

Assuming the metadata in the library has been produced from both the UDL and the proc-macros, the only possible fix is to not get metadata from parsing the UDL, aka. library mode.

mhammond commented 11 months ago

That seems correct for the current implementation, yeah. Isn't it getting easier to use library-mode?

mhammond commented 11 months ago

Sorry I didn't look closer before and that I focused on library mode, but yeah, this a bug in uniffi - we shouldn't end up with dupe traitmethods. And I agree that makes no sense - we must be ending up here twice, but we do the same dumb push elsewhere too. I can't see what's wrong.