Skip to content

Commit

Permalink
fix: Fix "legacy" enums breaking Swift compiler (#346)
Browse files Browse the repository at this point in the history
* feat: Add `OldEnum` tests (enum with numbers)

* whoops

* whoops 2

* fix: Fix `std::optional<enum>` breaking swift compiler

* Update SwiftCxxBridgedType.ts
  • Loading branch information
mrousavy authored Nov 19, 2024
1 parent c2becae commit cb9cf99
Show file tree
Hide file tree
Showing 22 changed files with 246 additions and 2 deletions.
17 changes: 17 additions & 0 deletions example/src/getTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,23 @@ export function getTests(
.didReturn('string')
.equals('gas')
),
createTest('get optionalOldEnum (== undefined)', () =>
it(() => {
testObject.optionalOldEnum = undefined
return testObject.optionalOldEnum
})
.didNotThrow()
.didReturn('undefined')
),
createTest('get optionalOldEnum (== self)', () =>
it(() => {
testObject.optionalOldEnum = OldEnum.SECOND
return testObject.optionalOldEnum
})
.didNotThrow()
.didReturn(typeof OldEnum.SECOND)
.equals(OldEnum.SECOND)
),

// Test basic functions
createTest('addNumbers(5, 13) = 18', () =>
Expand Down
12 changes: 11 additions & 1 deletion packages/nitrogen/src/syntax/swift/SwiftCxxBridgedType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
language: 'swift' | 'c++'
): string {
switch (this.type.kind) {
case 'enum':
case 'enum': {
if (this.isBridgingToDirectCppTarget) {
return cppParameterName
}
Expand All @@ -293,6 +293,7 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
default:
throw new Error(`Invalid language! ${language}`)
}
}
case 'hybrid-object': {
const bridge = this.getBridgeOrThrow()
const getFunc = `bridge.get_${bridge.specializationName}`
Expand Down Expand Up @@ -340,6 +341,15 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
const wrapping = new SwiftCxxBridgedType(optional.wrappingType, true)
switch (language) {
case 'swift':
if (wrapping.type.kind === 'enum') {
const enumType = getTypeAs(wrapping.type, EnumType)
if (enumType.jsType === 'enum') {
// TODO: Remove this hack once Swift fixes this shit.
// A JS enum is implemented as a number/int based enum.
// For some reason, those break in Swift. I have no idea why.
return `${cppParameterName}.has_value() ? ${cppParameterName}.pointee : nil`
}
}
if (!wrapping.needsSpecialHandling) {
return `${cppParameterName}.value`
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class HybridTestObjectKotlin: HybridTestObjectSwiftKotlinSpec() {
override var someVariant: Variant_String_Double = Variant_String_Double.create(55.05)
override var optionalArray: Array<String>? = null
override var optionalEnum: Powertrain? = null
override var optionalOldEnum: OldEnum? = null

override fun simpleFunc() {
// do nothing
Expand Down
8 changes: 8 additions & 0 deletions packages/react-native-nitro-image/cpp/HybridTestObjectCpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ void HybridTestObjectCpp::setOptionalEnum(std::optional<Powertrain> optionalEnum
_optionalEnum = optionalEnum;
}

std::optional<OldEnum> HybridTestObjectCpp::getOptionalOldEnum() {
return _optionalOldEnum;
}

void HybridTestObjectCpp::setOptionalOldEnum(std::optional<OldEnum> optionalOldEnum) {
_optionalOldEnum = optionalOldEnum;
}

// Methods
double HybridTestObjectCpp::addNumbers(double a, double b) {
return a + b;
Expand Down
3 changes: 3 additions & 0 deletions packages/react-native-nitro-image/cpp/HybridTestObjectCpp.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
std::optional<std::vector<std::string>> _optionalArray;
std::optional<std::shared_ptr<HybridTestObjectCppSpec>> _optionalHybrid;
std::optional<Powertrain> _optionalEnum;
std::optional<OldEnum> _optionalOldEnum;

private:
static inline uint64_t calculateFibonacci(int count) noexcept {
Expand Down Expand Up @@ -67,6 +68,8 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
void setOptionalHybrid(const std::optional<std::shared_ptr<HybridTestObjectCppSpec>>& optionalHybrid) override;
std::optional<Powertrain> getOptionalEnum() override;
void setOptionalEnum(std::optional<Powertrain> optionalEnum) override;
std::optional<OldEnum> getOptionalOldEnum() override;
void setOptionalOldEnum(std::optional<OldEnum> optionalOldEnum) override;

public:
// Methods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class HybridTestObjectSwift : HybridTestObjectSwiftKotlinSpec {

var optionalEnum: Powertrain? = nil

var optionalOldEnum: OldEnum? = nil

func simpleFunc() throws {
// do nothing
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }
// Forward declaration of `OldEnum` to properly resolve imports.
namespace margelo::nitro::image { enum class OldEnum; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `AnyMap` to properly resolve imports.
Expand All @@ -33,6 +35,8 @@ namespace margelo::nitro::image { class HybridBaseSpec; }
#include <vector>
#include "Powertrain.hpp"
#include "JPowertrain.hpp"
#include "OldEnum.hpp"
#include "JOldEnum.hpp"
#include <variant>
#include "JVariant_String_Double.hpp"
#include "Person.hpp"
Expand Down Expand Up @@ -184,6 +188,15 @@ namespace margelo::nitro::image {
static const auto method = _javaPart->getClass()->getMethod<void(jni::alias_ref<JPowertrain> /* optionalEnum */)>("setOptionalEnum");
method(_javaPart, optionalEnum.has_value() ? JPowertrain::fromCpp(optionalEnum.value()) : nullptr);
}
std::optional<OldEnum> JHybridTestObjectSwiftKotlinSpec::getOptionalOldEnum() {
static const auto method = _javaPart->getClass()->getMethod<jni::local_ref<JOldEnum>()>("getOptionalOldEnum");
auto __result = method(_javaPart);
return __result != nullptr ? std::make_optional(__result->toCpp()) : std::nullopt;
}
void JHybridTestObjectSwiftKotlinSpec::setOptionalOldEnum(std::optional<OldEnum> optionalOldEnum) {
static const auto method = _javaPart->getClass()->getMethod<void(jni::alias_ref<JOldEnum> /* optionalOldEnum */)>("setOptionalOldEnum");
method(_javaPart, optionalOldEnum.has_value() ? JOldEnum::fromCpp(optionalOldEnum.value()) : nullptr);
}
std::variant<std::string, double> JHybridTestObjectSwiftKotlinSpec::getSomeVariant() {
static const auto method = _javaPart->getClass()->getMethod<jni::local_ref<JVariant_String_Double>()>("getSomeVariant");
auto __result = method(_javaPart);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ namespace margelo::nitro::image {
void setOptionalArray(const std::optional<std::vector<std::string>>& optionalArray) override;
std::optional<Powertrain> getOptionalEnum() override;
void setOptionalEnum(std::optional<Powertrain> optionalEnum) override;
std::optional<OldEnum> getOptionalOldEnum() override;
void setOptionalOldEnum(std::optional<OldEnum> optionalOldEnum) override;
std::variant<std::string, double> getSomeVariant() override;
void setSomeVariant(const std::variant<std::string, double>& someVariant) override;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
///
/// JOldEnum.hpp
/// This file was generated by nitrogen. DO NOT MODIFY THIS FILE.
/// https://github.com/mrousavy/nitro
/// Copyright © 2024 Marc Rousavy @ Margelo
///

#pragma once

#include <fbjni/fbjni.h>
#include "OldEnum.hpp"

namespace margelo::nitro::image {

using namespace facebook;

/**
* The C++ JNI bridge between the C++ enum "OldEnum" and the the Kotlin enum "OldEnum".
*/
struct JOldEnum final: public jni::JavaClass<JOldEnum> {
public:
static auto constexpr kJavaDescriptor = "Lcom/margelo/nitro/image/OldEnum;";

public:
/**
* Convert this Java/Kotlin-based enum to the C++ enum OldEnum.
*/
[[maybe_unused]]
OldEnum toCpp() const {
static const auto clazz = javaClassStatic();
static const auto fieldOrdinal = clazz->getField<int>("ordinal");
int ordinal = this->getFieldValue(fieldOrdinal);
return static_cast<OldEnum>(ordinal);
}

public:
/**
* Create a Java/Kotlin-based enum with the given C++ enum's value.
*/
[[maybe_unused]]
static jni::alias_ref<JOldEnum> fromCpp(OldEnum value) {
static const auto clazz = javaClassStatic();
static const auto fieldFIRST = clazz->getStaticField<JOldEnum>("FIRST");
static const auto fieldSECOND = clazz->getStaticField<JOldEnum>("SECOND");
static const auto fieldTHIRD = clazz->getStaticField<JOldEnum>("THIRD");

switch (value) {
case OldEnum::FIRST:
return clazz->getStaticFieldValue(fieldFIRST);
case OldEnum::SECOND:
return clazz->getStaticFieldValue(fieldSECOND);
case OldEnum::THIRD:
return clazz->getStaticFieldValue(fieldTHIRD);
default:
std::string stringValue = std::to_string(static_cast<int>(value));
throw std::invalid_argument("Invalid enum value (" + stringValue + "!");
}
}
};

} // namespace margelo::nitro::image
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ abstract class HybridTestObjectSwiftKotlinSpec: HybridObject() {
@set:Keep
abstract var optionalEnum: Powertrain?

@get:DoNotStrip
@get:Keep
@set:DoNotStrip
@set:Keep
abstract var optionalOldEnum: OldEnum?

@get:DoNotStrip
@get:Keep
@set:DoNotStrip
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
///
/// OldEnum.kt
/// This file was generated by nitrogen. DO NOT MODIFY THIS FILE.
/// https://github.com/mrousavy/nitro
/// Copyright © 2024 Marc Rousavy @ Margelo
///

package com.margelo.nitro.image

import androidx.annotation.Keep
import com.facebook.proguard.annotations.DoNotStrip

/**
* Represents the JavaScript enum/union "OldEnum".
*/
@DoNotStrip
@Keep
enum class OldEnum {
FIRST,
SECOND,
THIRD
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace margelo::nitro::image { class HybridImageFactorySpec; }
namespace margelo::nitro::image { class HybridImageSpec; }
// Forward declaration of `HybridTestObjectSwiftKotlinSpec` to properly resolve imports.
namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
// Forward declaration of `OldEnum` to properly resolve imports.
namespace margelo::nitro::image { enum class OldEnum; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `Powertrain` to properly resolve imports.
Expand All @@ -45,6 +47,7 @@ namespace NitroImage { class HybridTestObjectSwiftKotlinSpecCxx; }
#include "HybridImageFactorySpec.hpp"
#include "HybridImageSpec.hpp"
#include "HybridTestObjectSwiftKotlinSpec.hpp"
#include "OldEnum.hpp"
#include "Person.hpp"
#include "Powertrain.hpp"
#include <NitroModules/ArrayBuffer.hpp>
Expand Down Expand Up @@ -163,6 +166,15 @@ namespace margelo::nitro::image::bridge::swift {
return std::optional<Powertrain>(value);
}

// pragma MARK: std::optional<OldEnum>
/**
* Specialized version of `std::optional<OldEnum>`.
*/
using std__optional_OldEnum_ = std::optional<OldEnum>;
inline std::optional<OldEnum> create_std__optional_OldEnum_(const OldEnum& value) {
return std::optional<OldEnum>(value);
}

// pragma MARK: std::vector<double>
/**
* Specialized version of `std::vector<double>`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
namespace margelo::nitro::image { enum class ImageFormat; }
// Forward declaration of `ImageSize` to properly resolve imports.
namespace margelo::nitro::image { struct ImageSize; }
// Forward declaration of `OldEnum` to properly resolve imports.
namespace margelo::nitro::image { enum class OldEnum; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `PixelFormat` to properly resolve imports.
Expand All @@ -44,6 +46,7 @@ namespace margelo::nitro::image { enum class Powertrain; }
#include "HybridTestObjectSwiftKotlinSpec.hpp"
#include "ImageFormat.hpp"
#include "ImageSize.hpp"
#include "OldEnum.hpp"
#include "Person.hpp"
#include "PixelFormat.hpp"
#include "Powertrain.hpp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace NitroImage { class HybridTestObjectSwiftKotlinSpecCxx; }
namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }
// Forward declaration of `OldEnum` to properly resolve imports.
namespace margelo::nitro::image { enum class OldEnum; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `AnyMap` to properly resolve imports.
Expand All @@ -37,6 +39,7 @@ namespace margelo::nitro::image { class HybridBaseSpec; }
#include <string>
#include <vector>
#include "Powertrain.hpp"
#include "OldEnum.hpp"
#include <variant>
#include "Person.hpp"
#include <functional>
Expand Down Expand Up @@ -159,6 +162,13 @@ namespace margelo::nitro::image {
inline void setOptionalEnum(std::optional<Powertrain> optionalEnum) noexcept override {
_swiftPart.setOptionalEnum(optionalEnum);
}
inline std::optional<OldEnum> getOptionalOldEnum() noexcept override {
auto __result = _swiftPart.getOptionalOldEnum();
return __result;
}
inline void setOptionalOldEnum(std::optional<OldEnum> optionalOldEnum) noexcept override {
_swiftPart.setOptionalOldEnum(optionalOldEnum);
}
inline std::variant<std::string, double> getSomeVariant() noexcept override {
auto __result = _swiftPart.getSomeVariant();
return __result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public protocol HybridTestObjectSwiftKotlinSpec: AnyObject, HybridObjectSpec {
var optionalString: String? { get set }
var optionalArray: [String]? { get set }
var optionalEnum: Powertrain? { get set }
var optionalOldEnum: OldEnum? { get set }
var someVariant: Variant_String_Double { get set }

// Methods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,23 @@ public class HybridTestObjectSwiftKotlinSpecCxx {
}
}

public var optionalOldEnum: bridge.std__optional_OldEnum_ {
@inline(__always)
get {
return { () -> bridge.std__optional_OldEnum_ in
if let __unwrappedValue = self.__implementation.optionalOldEnum {
return bridge.create_std__optional_OldEnum_(__unwrappedValue)
} else {
return .init()
}
}()
}
@inline(__always)
set {
self.__implementation.optionalOldEnum = newValue.has_value() ? newValue.pointee : nil
}
}

public var someVariant: bridge.std__variant_std__string__double_ {
@inline(__always)
get {
Expand Down
Loading

0 comments on commit cb9cf99

Please sign in to comment.