Skip to content

Commit c136f5f

Browse files
mrousavymrousavy
authored
fix: Fix nested arrays auto __element = __element assignment (#1307)
* chore: Add test for nested arrays * impl test * format * Update Podfile.lock * fix: Input `__input` to lambda so `__element` isn't shadowed
1 parent 9842816 commit c136f5f

24 files changed

Lines changed: 301 additions & 149 deletions

‎example/ios/Podfile.lock‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2217,7 +2217,7 @@ EXTERNAL SOURCES:
22172217

22182218
SPEC CHECKSUMS:
22192219
FBLazyVector: 7c1d69992204c5ec452eeefa4a24b0ff311709c8
2220-
hermes-engine: 16e781d7fca74c8bb3ca59b99527d9633ee9ee36
2220+
hermes-engine: b69aade2dcf59e8c2203c748ca79c7fa4c875053
22212221
NitroModules: 2ece7b1523f5e1952c5bcea7c318df11b8ac856b
22222222
NitroTest: 63853fc7241553f36a2ed2a9c6ec56c35f1408a8
22232223
NitroTestExternal: 17a3bd766b3ee6f122d9857b575f0a27fb013d53
@@ -2229,7 +2229,7 @@ SPEC CHECKSUMS:
22292229
React: f6f8fc5c01e77349cdfaf49102bcb928ac31d8ed
22302230
React-callinvoker: 3e62a849bda1522c6422309c02f5dc3210bc5359
22312231
React-Core: 0b765bc7c2b83dff178c2b03ed8a0390df26f18c
2232-
React-Core-prebuilt: 88810feb58457484bff17e9e91a15453407d745a
2232+
React-Core-prebuilt: 1ed3b91c7035f715bf741f1b8a2c87e94d126d38
22332233
React-CoreModules: 55b932564ba696301cb683a86385be6a6f137e57
22342234
React-cxxreact: 5bfb95256fde56cc0f9ce425f38cfaa085e71ad2
22352235
React-debug: f9dda2791d3ebe2078bc6102641edab77917efb7
@@ -2292,7 +2292,7 @@ SPEC CHECKSUMS:
22922292
ReactAppDependencyProvider: 23e2bca1661f8781e55fcc05a151fc1df97bc1fb
22932293
ReactCodegen: c049d7e966ed24be56d8e21cb1b8880316975e76
22942294
ReactCommon: 89ccc6cb100ca5a0303b46483037ef8f3e06e2e0
2295-
ReactNativeDependencies: 1a7e3c3ffa57533d8118dd9bc01790ffa9e02a3b
2295+
ReactNativeDependencies: 69de62c8a59d21e7b6af8bdfb62b61e798f65351
22962296
RNScreens: dd61bc3a3e6f6901ad833efa411917d44827cf51
22972297
Yoga: 21f482cbc18b56cdc477cd3a0c5b8c2c83ac27ce
22982298

‎example/src/getTests.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,28 @@ export function getTests(
584584
{ age: 5, name: 'Ben' },
585585
])
586586
),
587+
createTest('bounceNestedArray(...) equals', () =>
588+
it(() =>
589+
testObject.bounceNestedArray([
590+
[
591+
{ age: 24, name: 'Marc' },
592+
{ age: 5, name: 'Ben' },
593+
],
594+
[{ age: 30, name: 'Anna' }],
595+
[],
596+
])
597+
)
598+
.didNotThrow()
599+
.didReturn('object')
600+
.equals([
601+
[
602+
{ age: 24, name: 'Marc' },
603+
{ age: 5, name: 'Ben' },
604+
],
605+
[{ age: 30, name: 'Anna' }],
606+
[],
607+
])
608+
),
587609
createTest('bouncePartialStruct(...) empty equals', () =>
588610
it(() =>
589611
testObject.bouncePartialStruct({ name: undefined, age: undefined })

‎packages/nitrogen/src/syntax/kotlin/KotlinCxxBridgedType.ts‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -672,18 +672,24 @@ export class KotlinCxxBridgedType implements BridgedType<'kotlin', 'c++'> {
672672
`.trim()
673673
}
674674
default: {
675-
// other arrays need to loop through
675+
// other arrays need to loop through.
676+
// we wrap the lambda with a parameter (`__input`) so that nested arrays
677+
// don't end up shadowing `__element` in their own initializers
678+
// (which would be `const auto& __element = __element[__i]`,
679+
// an invalid self-referential `auto`-type deduction).
680+
// `auto&&` is a forwarding reference so it binds to both const and
681+
// non-const inputs across nesting levels.
676682
return `
677-
[&]() {
678-
size_t __size = ${parameterName}.size();
683+
[&](auto&& __input) {
684+
size_t __size = __input.size();
679685
jni::local_ref<${arrayType}> __array = ${arrayType}::newArray(__size);
680686
for (size_t __i = 0; __i < __size; __i++) {
681-
const auto& __element = ${parameterName}[__i];
687+
const auto& __element = __input[__i];
682688
auto __elementJni = ${indent(bridge.parseFromCppToKotlin('__element', 'c++'), ' ')};
683689
__array->setElement(__i, ${bridge.dereferenceToJObject('__elementJni')});
684690
}
685691
return __array;
686-
}()
692+
}(${parameterName})
687693
`.trim()
688694
}
689695
}
@@ -976,18 +982,25 @@ export class KotlinCxxBridgedType implements BridgedType<'kotlin', 'c++'> {
976982
`.trim()
977983
}
978984
default: {
979-
// other arrays need to loop through
985+
// other arrays need to loop through.
986+
// we wrap the lambda with a parameter (`__input`) so that nested arrays
987+
// don't end up shadowing `__element` in their own initializers
988+
// (which would be `auto __element = __element->getElement(__i)`,
989+
// an invalid self-referential `auto`-type deduction).
990+
// `auto&&` is a forwarding reference so it binds to both const and
991+
// non-const inputs across nesting levels (and to JNI `local_ref`s,
992+
// which expose non-const `getElement(...)`).
980993
return `
981-
[&]() {
982-
size_t __size = ${parameterName}->size();
994+
[&](auto&& __input) {
995+
size_t __size = __input->size();
983996
std::vector<${itemType}> __vector;
984997
__vector.reserve(__size);
985998
for (size_t __i = 0; __i < __size; __i++) {
986-
auto __element = ${parameterName}->getElement(__i);
987-
__vector.push_back(${bridge.parseFromKotlinToCpp('__element', 'c++')});
999+
auto __element = __input->getElement(__i);
1000+
__vector.push_back(${indent(bridge.parseFromKotlinToCpp('__element', 'c++'), ' ')});
9881001
}
9891002
return __vector;
990-
}()
1003+
}(${parameterName})
9911004
`.trim()
9921005
}
9931006
}

‎packages/react-native-nitro-test/android/src/main/java/com/margelo/nitro/test/HybridTestObjectKotlin.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ class HybridTestObjectKotlin : HybridTestObjectSwiftKotlinSpec() {
8383
return array
8484
}
8585

86+
override fun bounceNestedArray(array: Array<Array<Person>>): Array<Array<Person>> {
87+
return array
88+
}
89+
8690
override fun bouncePartialStruct(person: PartialPerson): PartialPerson {
8791
return person
8892
}

‎packages/react-native-nitro-test/cpp/HybridTestObjectCpp.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,10 @@ std::vector<Person> HybridTestObjectCpp::bounceStructs(const std::vector<Person>
216216
return array;
217217
}
218218

219+
std::vector<std::vector<Person>> HybridTestObjectCpp::bounceNestedArray(const std::vector<std::vector<Person>>& array) {
220+
return array;
221+
}
222+
219223
PartialPerson HybridTestObjectCpp::bouncePartialStruct(const PartialPerson& person) {
220224
return person;
221225
}

‎packages/react-native-nitro-test/cpp/HybridTestObjectCpp.hpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
127127
std::vector<std::string> bounceStrings(const std::vector<std::string>& array) override;
128128
std::vector<double> bounceNumbers(const std::vector<double>& array) override;
129129
std::vector<Person> bounceStructs(const std::vector<Person>& array) override;
130+
std::vector<std::vector<Person>> bounceNestedArray(const std::vector<std::vector<Person>>& array) override;
130131
PartialPerson bouncePartialStruct(const PartialPerson& person) override;
131132
std::string sumUpAllPassengers(const std::vector<Car>& cars) override;
132133
std::vector<Powertrain> bounceEnums(const std::vector<Powertrain>& array) override;

‎packages/react-native-nitro-test/ios/HybridTestObjectSwift.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,10 @@ class HybridTestObjectSwift: HybridTestObjectSwiftKotlinSpec {
178178
return array
179179
}
180180

181+
func bounceNestedArray(array: [[Person]]) throws -> [[Person]] {
182+
return array
183+
}
184+
181185
func bouncePartialStruct(person: PartialPerson) throws -> PartialPerson {
182186
return person
183187
}

‎packages/react-native-nitro-test/nitrogen/generated/android/c++/JCar.hpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,16 @@ namespace margelo::nitro::test {
6868
power,
6969
powertrain->toCpp(),
7070
driver != nullptr ? std::make_optional(driver->toCpp()) : std::nullopt,
71-
[&]() {
72-
size_t __size = passengers->size();
71+
[&](auto&& __input) {
72+
size_t __size = __input->size();
7373
std::vector<Person> __vector;
7474
__vector.reserve(__size);
7575
for (size_t __i = 0; __i < __size; __i++) {
76-
auto __element = passengers->getElement(__i);
76+
auto __element = __input->getElement(__i);
7777
__vector.push_back(__element->toCpp());
7878
}
7979
return __vector;
80-
}(),
80+
}(passengers),
8181
isFast != nullptr ? std::make_optional(static_cast<bool>(isFast->value())) : std::nullopt,
8282
favouriteTrack != nullptr ? std::make_optional(favouriteTrack->toStdString()) : std::nullopt,
8383
[&]() {
@@ -107,16 +107,16 @@ namespace margelo::nitro::test {
107107
value.power,
108108
JPowertrain::fromCpp(value.powertrain),
109109
value.driver.has_value() ? JPerson::fromCpp(value.driver.value()) : nullptr,
110-
[&]() {
111-
size_t __size = value.passengers.size();
110+
[&](auto&& __input) {
111+
size_t __size = __input.size();
112112
jni::local_ref<jni::JArrayClass<JPerson>> __array = jni::JArrayClass<JPerson>::newArray(__size);
113113
for (size_t __i = 0; __i < __size; __i++) {
114-
const auto& __element = value.passengers[__i];
114+
const auto& __element = __input[__i];
115115
auto __elementJni = JPerson::fromCpp(__element);
116116
__array->setElement(__i, *__elementJni);
117117
}
118118
return __array;
119-
}(),
119+
}(value.passengers),
120120
value.isFast.has_value() ? jni::JBoolean::valueOf(value.isFast.value()) : nullptr,
121121
value.favouriteTrack.has_value() ? jni::make_jstring(value.favouriteTrack.value()) : nullptr,
122122
[&]() {

‎packages/react-native-nitro-test/nitrogen/generated/android/c++/JFunc_void_std__vector_Powertrain_.hpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,16 @@ namespace margelo::nitro::test {
3434
*/
3535
void invoke(const std::vector<Powertrain>& array) const {
3636
static const auto method = javaClassStatic()->getMethod<void(jni::alias_ref<jni::JArrayClass<JPowertrain>> /* array */)>("invoke");
37-
method(self(), [&]() {
38-
size_t __size = array.size();
37+
method(self(), [&](auto&& __input) {
38+
size_t __size = __input.size();
3939
jni::local_ref<jni::JArrayClass<JPowertrain>> __array = jni::JArrayClass<JPowertrain>::newArray(__size);
4040
for (size_t __i = 0; __i < __size; __i++) {
41-
const auto& __element = array[__i];
41+
const auto& __element = __input[__i];
4242
auto __elementJni = JPowertrain::fromCpp(__element);
4343
__array->setElement(__i, *__elementJni);
4444
}
4545
return __array;
46-
}());
46+
}(array));
4747
}
4848
};
4949

@@ -61,16 +61,16 @@ namespace margelo::nitro::test {
6161
* Invokes the C++ `std::function<...>` this `JFunc_void_std__vector_Powertrain__cxx` instance holds.
6262
*/
6363
void invoke_cxx(jni::alias_ref<jni::JArrayClass<JPowertrain>> array) {
64-
_func([&]() {
65-
size_t __size = array->size();
64+
_func([&](auto&& __input) {
65+
size_t __size = __input->size();
6666
std::vector<Powertrain> __vector;
6767
__vector.reserve(__size);
6868
for (size_t __i = 0; __i < __size; __i++) {
69-
auto __element = array->getElement(__i);
69+
auto __element = __input->getElement(__i);
7070
__vector.push_back(__element->toCpp());
7171
}
7272
return __vector;
73-
}());
73+
}(array));
7474
}
7575

7676
public:

0 commit comments

Comments
 (0)