Skip to content

Commit

Permalink
Add type collection pass to get more info about External types (#66)
Browse files Browse the repository at this point in the history
According to [The Big O of Code
Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a
O(_n_) change.

This is a large diff, but most of the work is mechanical.

1. Allow the test-runner to initialize more than one C++ file. 
2. Adding a lookup for `External` types so the templates have more
detail about what the `External` (imported) types actually are.
3. Adding a `ext-types` fixture.

The most interesting of these is the second:

Before rendering any files, a pass is taken of all
`ComponentInterface`s.

Each type is collected, and added to a type map, keyed by module-path
and name.

When an external type is encountered later, the type map is consulted,
and the actual type
is used.

This is to remove a deficiency in `Type::External`: the `ExternalKind`
enum did not map completely onto a `Type`:
before this change, it wasn't clear what sort of type was being
imported, and so it was difficult/impossible
to work out __how__ to import (`import { X }` vs `import { type X }`),
or __what__ to import: `X` vs `XInterface`.

This meant changing the `type_name`, `ffi_converter_name` and some other
filter methods, necessitating __many__ template files. These changes can
be largely skimmed over.

The test also caught a runtime bug, which is also fixed in this PR: when
a Rust trait is implemented by Rust, but exposed (as a typescript
`interface`), this was erroring at runtime with a
`UniffiStaleHandleError`. This is now fixed.
  • Loading branch information
jhugman authored Aug 16, 2024
1 parent 12a0ee0 commit 1b48cc5
Show file tree
Hide file tree
Showing 56 changed files with 1,391 additions and 167 deletions.
2 changes: 1 addition & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[alias]
xtask = "run --package xtask --"
xtask = "--quiet run --package xtask --"
1 change: 0 additions & 1 deletion cpp/hermes-extension/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,4 @@ link_directories("${HERMES_BUILD_DIR}/API/hermes")
link_directories("${HERMES_BUILD_DIR}/jsi")
link_libraries(jsi)

add_definitions(-DUNIFFI_ENABLE_TEST_HOOKS)
add_library(${HERMES_EXTENSION_NAME} SHARED ${HERMES_EXTENSION_CPP})
1 change: 0 additions & 1 deletion cpp/hermes-rust-extension/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,5 @@ link_directories("${HERMES_BUILD_DIR}/jsi")
link_libraries(jsi)
link_directories("${RUST_TARGET_DIR}")

add_definitions(-DUNIFFI_ENABLE_TEST_HOOKS)
add_library(${HERMES_EXTENSION_NAME} SHARED ${HERMES_EXTENSION_CPP})
target_link_libraries(${HERMES_EXTENSION_NAME} ${RUST_LIB_NAME})
17 changes: 16 additions & 1 deletion crates/ubrn_bindgen/src/bindings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@

pub mod metadata;
pub(crate) mod react_native;
pub(crate) mod type_map;

use std::str::FromStr;
use std::{fs, str::FromStr};

use anyhow::Result;
use askama::Template;
use camino::{Utf8Path, Utf8PathBuf};
use clap::{command, Args};
use ubrn_common::mk_dir;
Expand Down Expand Up @@ -150,4 +152,17 @@ impl BindingsArgs {

Ok(configs)
}

pub fn render_entrypoint(&self, path: &Utf8Path, modules: &Vec<ModuleMetadata>) -> Result<()> {
let index = EntrypointCpp { modules };
let string = index.render()?;
fs::write(path, string)?;
Ok(())
}
}

#[derive(Template)]
#[template(path = "entrypoint.cpp", escape = "none")]
struct EntrypointCpp<'a> {
modules: &'a Vec<ModuleMetadata>,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// This file was autogenerated by some hot garbage in the `uniffi-bindgen-react-native` crate.
// Trust me, you don't want to mess with it!

// This template is only used when running `cargo xtask run`, i.e. when running `./scripts/run-tests.sh`.
// It is an implementation of the `registerNatives` function which is used by the test-runner to dynamically
// load the native modules without re-compiling the test-runner.
//
// Files generated with this template appear in the fixtures/{fixture}/generated/cpp directory, always called
// `Entrypoint.cpp`, with a captilized `E` to ensure it does not collide with namespaces called "entrypoint".

#include "registerNatives.h"

{%- for m in modules %}
#include "{{ m.hpp_filename() }}";
{%- endfor %}

extern "C" void registerNatives(jsi::Runtime &rt, std::shared_ptr<react::CallInvoker> callInvoker) {
{%- for m in modules %}
{{ m.cpp_module() }}::registerModule(rt, callInvoker);
{%- endfor %}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{%- let namespace = ci.namespace() %}
{%- let module_name = module.cpp_module() %}
// This file was autogenerated by some hot garbage in the `uniffi-bindgen-react-native` crate.
// Trust me, you don't want to mess with it!
{%- let namespace = ci.namespace() %}
{%- let module_name = module.cpp_module() %}
#include "{{ module.hpp_filename() }}"

#include "UniffiJsiTypes.h"
Expand All @@ -14,14 +14,6 @@
namespace react = facebook::react;
namespace jsi = facebook::jsi;

#ifdef UNIFFI_ENABLE_TEST_HOOKS
// Initialization into the Hermes Runtime
#include "registerNatives.h"
extern "C" void registerNatives(jsi::Runtime &rt, std::shared_ptr<react::CallInvoker> callInvoker) {
{{ module_name }}::registerModule(rt, callInvoker);
}
#endif

// Calling into Rust.
extern "C" {
{%- for definition in ci.ffi_definitions() %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{%- let namespace = ci.namespace() %}
{%- let module_name = module.cpp_module() %}
// This file was autogenerated by some hot garbage in the `uniffi-bindgen-react-native` crate.
// Trust me, you don't want to mess with it!
{%- let namespace = ci.namespace() %}
{%- let module_name = module.cpp_module() %}
#pragma once
#include <jsi/jsi.h>
#include <iostream>
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/
*/
use super::oracle::{AsCodeType, CodeOracle};
use super::{
oracle::{AsCodeType, CodeOracle},
TypeRenderer,
};
pub(crate) use uniffi_bindgen::backend::filters::*;
use uniffi_bindgen::{
backend::{Literal, Type},
Expand All @@ -12,47 +15,72 @@ use uniffi_bindgen::{
};

pub(super) fn type_name(
as_ct: &impl AsCodeType,
ci: &ComponentInterface,
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
Ok(as_ct.as_codetype().type_label(ci))
let type_ = types.as_type(as_type);
Ok(type_.as_codetype().type_label(types.ci))
}

pub(super) fn decl_type_name(
as_ct: &impl AsCodeType,
ci: &ComponentInterface,
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
Ok(as_ct.as_codetype().decl_type_label(ci))
}

pub(super) fn canonical_name(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
Ok(as_ct.as_codetype().canonical_name())
let type_ = types.as_type(as_type);
Ok(type_.as_codetype().decl_type_label(types.ci))
}

pub(super) fn ffi_converter_name(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
Ok(as_ct.as_codetype().ffi_converter_name())
pub(super) fn ffi_converter_name(
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
let type_ = types.as_type(as_type);
Ok(type_.as_codetype().ffi_converter_name())
}

pub(super) fn ffi_error_converter_name(as_type: &impl AsType) -> Result<String, askama::Error> {
pub(super) fn ffi_error_converter_name(
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
// special handling for types used as errors.
let mut name = ffi_converter_name(as_type)?;
if matches!(&as_type.as_type(), Type::Object { .. }) {
let type_ = types.as_type(as_type);
let mut name = type_.as_codetype().ffi_converter_name();
if matches!(type_, Type::Object { .. }) {
name.push_str("__as_error")
}
if let Type::External { namespace, .. } = as_type.as_type() {
types.import_converter(name.clone(), &namespace);
}
Ok(name)
}

pub(super) fn lower_fn(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
pub(super) fn lower_error_fn(
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
Ok(format!(
"{ct}.lower.bind({ct})",
ct = as_ct.as_codetype().ffi_converter_name()
ct = ffi_error_converter_name(as_type, types)?
))
}

pub(super) fn lift_error_fn(
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
Ok(format!(
"{ct}.lift.bind({ct})",
ct = ffi_error_converter_name(as_type, types)?
))
}

pub(super) fn lift_fn(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
pub(super) fn lift_fn(
as_type: &impl AsType,
types: &TypeRenderer,
) -> Result<String, askama::Error> {
Ok(format!(
"{ct}.lift.bind({ct})",
ct = as_ct.as_codetype().ffi_converter_name()
ct = ffi_converter_name(as_type, types)?
))
}

Expand Down
Loading

0 comments on commit 1b48cc5

Please sign in to comment.