Skip to content

Generate generics for ExistingRustTypes #579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Generate generics for ExistingRustTypes #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions common/annotation_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ static absl::Status InconsistentAnnotationsError(
static absl::Status CheckExpressionsAreSameConstant(
const clang::Expr& expr1, const clang::Expr& expr2,
absl::string_view annotation_name, const clang::ASTContext& ast_context) {
if (expr1.getType() != expr2.getType()) {
return InconsistentAnnotationsError(annotation_name);
}

clang::Expr::EvalResult eval_result_1, eval_result_2;
if (!expr1.EvaluateAsConstantExpr(eval_result_1, ast_context) ||
!expr2.EvaluateAsConstantExpr(eval_result_2, ast_context)) {
Expand All @@ -122,9 +126,10 @@ static absl::Status CheckExpressionsAreSameConstant(
}

auto must_be_int_or_string_error = [annotation_name]() {
return absl::InvalidArgumentError(absl::StrCat(
"Arguments of `", annotation_name,
"` annotation must be of integral type or string literals."));
return absl::InvalidArgumentError(
absl::StrCat("Arguments of `", annotation_name,
"` annotation must be of integral type, string literal, "
"or constructor."));
};

switch (value1.getKind()) {
Expand All @@ -133,6 +138,10 @@ static absl::Status CheckExpressionsAreSameConstant(
return InconsistentAnnotationsError(annotation_name);
}
break;
case clang::APValue::Struct:
// Structs only appear when we use them for their type info, they
// are all empty though.
break;
case clang::APValue::LValue: {
CRUBIT_ASSIGN_OR_RETURN(
absl::string_view value1_string,
Expand Down
9 changes: 5 additions & 4 deletions common/annotation_reader_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ TEST(AnnotationReaderTest, GetAnnotateAttrFailureArgNotIntegralOrString) {

auto& var = LookupDecl<clang::VarDecl>(ast.context(), "i");

ASSERT_THAT(GetAnnotateAttrArgs(var, "foo"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("Arguments of `foo` annotation must be of "
"integral type or string literals")));
ASSERT_THAT(
GetAnnotateAttrArgs(var, "foo"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("Arguments of `foo` annotation must be of integral "
"type, string literal, or constructor")));
}

TEST(AnnotationReaderTest, GetAnnotateAttrSuccessConsistentAnnotations) {
Expand Down
2 changes: 1 addition & 1 deletion rs_bindings_from_cc/generate_bindings/cpp_type_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub fn format_cpp_type_inner(
}
RsTypeKind::Primitive(primitive) => Ok(quote! { #primitive }),
RsTypeKind::BridgeType { original_type, .. } => cpp_type_name_for_record(original_type, ir),
RsTypeKind::ExistingRustType(existing_rust_type) => tagless_cpp_type_name_for_item(
RsTypeKind::ExistingRustType { existing_rust_type, .. } => tagless_cpp_type_name_for_item(
&Item::ExistingRustType(Rc::clone(existing_rust_type)),
ir,
),
Expand Down
106 changes: 78 additions & 28 deletions rs_bindings_from_cc/generate_bindings/database/rs_snippet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,17 @@ pub enum RsTypeKind {
/// This variant comes from the `CRUBIT_INTERNAL_RUST_TYPE` attribute macro in C++,
/// which is used on types like `SliceRef`, `StrRef`, and C++ types generated from Rust
/// types by cc_bindings_from_rs.
ExistingRustType(Rc<ExistingRustType>),
ExistingRustType {
existing_rust_type: Rc<ExistingRustType>,
template_args: Rc<[TemplateArg]>,
},
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum TemplateArg {
Type(RsTypeKind),
Int(i64),
Bool(bool),
}

/// Information about how the owned function object may be called.
Expand Down Expand Up @@ -816,7 +826,7 @@ impl RsTypeKind {
existing_rust_type.template_args.len()
);
};
let TemplateArg::Type(inner_cc_type) = template_arg else {
let ir::TemplateArg::Type(inner_cc_type) = template_arg else {
bail!("SliceRef should have a type as its singular template argument");
};

Expand All @@ -838,7 +848,25 @@ impl RsTypeKind {
});
}

Ok(RsTypeKind::ExistingRustType(existing_rust_type))
let template_args = existing_rust_type
.template_args
.iter()
.map(|template_arg| match template_arg {
ir::TemplateArg::Type(type_param) => {
let rs_type_kind = db.rs_type_kind(type_param.clone())?;
ensure!(
!rs_type_kind.is_bridge_type(),
"Type parameter cannot be a bridge type: {}",
rs_type_kind.display(db),
);
Ok(TemplateArg::Type(rs_type_kind))
}
ir::TemplateArg::Int(i) => Ok(TemplateArg::Int(*i)),
ir::TemplateArg::Bool(b) => Ok(TemplateArg::Bool(*b)),
})
.collect::<Result<Rc<[TemplateArg]>>>()?;

Ok(RsTypeKind::ExistingRustType { existing_rust_type, template_args })
}

/// Returns true if the type is known to be `Unpin`, false otherwise.
Expand Down Expand Up @@ -1011,7 +1039,9 @@ impl RsTypeKind {
require_feature(CrubitFeature::Supported, None)
}
}
RsTypeKind::ExistingRustType(_) => require_feature(CrubitFeature::Supported, None),
RsTypeKind::ExistingRustType { .. } => {
require_feature(CrubitFeature::Supported, None)
}
}
}
(missing_features, reasons.into_iter().join(", "))
Expand Down Expand Up @@ -1064,7 +1094,9 @@ impl RsTypeKind {
RsTypeKind::TypeAlias { .. } => unreachable!(),
RsTypeKind::Primitive(_) => true,
RsTypeKind::BridgeType { .. } => false,
RsTypeKind::ExistingRustType(existing_rust_type) => existing_rust_type.is_same_abi,
RsTypeKind::ExistingRustType { existing_rust_type, .. } => {
existing_rust_type.is_same_abi
}
}
}

Expand Down Expand Up @@ -1119,7 +1151,7 @@ impl RsTypeKind {
RsTypeKind::TypeAlias { .. } => unreachable!(),
RsTypeKind::Primitive(_) => true,
RsTypeKind::BridgeType { .. } => false,
RsTypeKind::ExistingRustType(_) => true,
RsTypeKind::ExistingRustType { .. } => true,
}
}

Expand Down Expand Up @@ -1227,7 +1259,7 @@ impl RsTypeKind {
}
BridgeRsTypeKind::C9Co { .. } => false,
},
RsTypeKind::ExistingRustType(_) => true,
RsTypeKind::ExistingRustType { .. } => true,
}
}

Expand Down Expand Up @@ -1421,7 +1453,7 @@ impl RsTypeKind {
}
}
RsTypeKind::BridgeType { .. } => self.to_token_stream(db),
RsTypeKind::ExistingRustType(_) => self.to_token_stream(db),
RsTypeKind::ExistingRustType { .. } => self.to_token_stream(db),
_ => self.to_token_stream(db),
}
}
Expand Down Expand Up @@ -1693,7 +1725,7 @@ impl RsTypeKind {

let generic_types_tokens =
generic_types.iter().map(|t| t.to_token_stream(db));
quote! { #path < #(#generic_types_tokens),* > }
quote! { #path::<#(#generic_types_tokens),*> }
}
BridgeRsTypeKind::ProtoMessageBridge { rust_name } => {
fully_qualify_type(db, ir::Item::Record(original_type.clone()), rust_name)
Expand Down Expand Up @@ -1733,11 +1765,26 @@ impl RsTypeKind {
}
}
}
RsTypeKind::ExistingRustType(existing_rust_type) => fully_qualify_type(
db,
ir::Item::ExistingRustType(existing_rust_type.clone()),
&existing_rust_type.rs_name,
),
RsTypeKind::ExistingRustType { existing_rust_type, template_args } => {
let path = fully_qualify_type(
db,
ir::Item::ExistingRustType(existing_rust_type.clone()),
&existing_rust_type.rs_name,
);

// If there are no template args, then we're done.
if template_args.is_empty() {
return path;
}

let template_args_tokens = template_args.iter().map(|t| match t {
TemplateArg::Type(rs_type_kind) => rs_type_kind.to_token_stream(db),
TemplateArg::Int(i) => quote! { #i },
TemplateArg::Bool(b) => quote! { #b },
});

quote! { #path::<#(#template_args_tokens),*> }
}
}
}
}
Expand Down Expand Up @@ -1881,7 +1928,7 @@ impl<'ty> Iterator for RsTypeKindIter<'ty> {
self.todo.push(result_type);
}
},
RsTypeKind::ExistingRustType(_) => {}
RsTypeKind::ExistingRustType { .. } => {}
};
Some(curr)
}
Expand All @@ -1898,18 +1945,21 @@ mod tests {
use token_stream_matchers::assert_rs_matches;

fn make_existing_rust_type(name: Rc<str>, is_same_abi: bool) -> RsTypeKind {
RsTypeKind::ExistingRustType(Rc::new(ExistingRustType {
rs_name: name.clone(),
cc_name: "".into(),
unique_name: name,
template_args: Vec::new(),
template_arg_names: Vec::new(),
owning_target: BazelLabel("//new/for/testing".into()),
size_align: None,
is_same_abi,
id: ItemId::new_for_testing(0),
must_bind: false,
}))
RsTypeKind::ExistingRustType {
existing_rust_type: Rc::new(ExistingRustType {
rs_name: name.clone(),
cc_name: "".into(),
unique_name: name,
template_args: Vec::new(),
template_arg_names: Vec::new(),
owning_target: BazelLabel("//new/for/testing".into()),
size_align: None,
is_same_abi,
id: ItemId::new_for_testing(0),
must_bind: false,
}),
template_args: Rc::default(),
}
}

#[gtest]
Expand All @@ -1930,7 +1980,7 @@ mod tests {
.dfs_iter()
.map(|t| match t {
RsTypeKind::FuncPtr { .. } => "fn".to_string(),
RsTypeKind::ExistingRustType(existing_rust_type) => {
RsTypeKind::ExistingRustType { existing_rust_type, .. } => {
existing_rust_type.rs_name.to_string()
}
_ => unreachable!("Only FuncPtr and ExistingRustType kinds are used in this test"),
Expand Down
121 changes: 121 additions & 0 deletions rs_bindings_from_cc/generate_bindings/generate_bindings_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use arc_anyhow::{anyhow, Result};
use database::code_snippet::BindingsTokens;
use database::rs_snippet::{Mutability, RsTypeKind};
use googletest::{expect_eq, gtest};
use ir_matchers::assert_ir_matches;
use ir_testing::{retrieve_func, with_lifetime_macros};
use multiplatform_ir_testing::{ir_from_cc, ir_from_cc_dependency};
use quote::quote;
Expand Down Expand Up @@ -1356,3 +1357,123 @@ fn test_existing_rust_type_assert_incomplete() -> Result<()> {
);
Ok(())
}

#[gtest]
fn test_existing_rust_type_reordered_template_args() -> Result<()> {
let ir = ir_from_cc(
r#" #pragma clang lifetime_elision
namespace crubit {
template <typename...>
struct crubit_internal_rust_type_args {};
}

template <typename T, typename U>
struct [[clang::annotate("crubit_internal_rust_type", "RustTwoArgs", crubit::crubit_internal_rust_type_args<T, U>())]] TwoArgs {};

template <typename T, typename U>
using Reordered = TwoArgs<U, T>;

void AcceptReordered(Reordered<int, float> x);
"#,
)?;

assert_ir_matches!(
ir,
quote! {
ExistingRustType {
rs_name: "RustTwoArgs", ...
template_args: [
Type(CcType {
variant: Primitive(Float),
...
}),
Type(CcType {
variant: Primitive(Int),
...
}),
],
...
}
}
);
Ok(())
}

#[gtest]
fn test_existing_rust_type_default_template_args() -> Result<()> {
let ir = ir_from_cc(
r#" #pragma clang lifetime_elision
namespace crubit {
template <typename...>
struct crubit_internal_rust_type_args {};
}

template <typename T, typename U = int>
struct [[clang::annotate("crubit_internal_rust_type", "RustTypeWithDefault", crubit::crubit_internal_rust_type_args<T, U>())]] WithDefault {};

void AcceptWithDefault(WithDefault<float> x);
"#,
)?;

assert_ir_matches!(
ir,
quote! {
ExistingRustType {
rs_name: "RustTypeWithDefault", ...
template_args: [
Type(CcType {
variant: Primitive(Float),
...
}),
Type(CcType {
variant: Primitive(Int),
...
}),
],
...
}
}
);
Ok(())
}

#[gtest]
fn test_existing_rust_type_specialized_template_args() -> Result<()> {
let ir = ir_from_cc(
r#" #pragma clang lifetime_elision
namespace crubit {
template <typename...>
struct crubit_internal_rust_type_args {};
}
template <typename T>
struct [[clang::annotate("crubit_internal_rust_type", "RustContainer", crubit::crubit_internal_rust_type_args<T>())]] Container {};

template <>
struct [[clang::annotate("crubit_internal_rust_type", "RustVoidContainer", crubit::crubit_internal_rust_type_args<>())]] Container<void> {};

void AcceptSpecialized(Container<float> a, Container<void> b);
"#,
)?;

assert_ir_matches!(
ir,
quote! {
ExistingRustType(ExistingRustType {
rs_name: "RustContainer", ...
template_args: [
Type(CcType {
variant: Primitive(Float),
...
})
],
...
}), ...
ExistingRustType(ExistingRustType {
rs_name: "RustVoidContainer", ...
template_args: [],
...
})
}
);
Ok(())
}
2 changes: 1 addition & 1 deletion rs_bindings_from_cc/generate_bindings/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ fn crubit_abi_type(db: &BindingsGenerator, rs_type_kind: RsTypeKind) -> Result<C

Ok(CrubitAbiType::Transmute { rust_type: rs_type_kind.to_token_stream(db), cpp_type })
}
RsTypeKind::ExistingRustType(ref existing_rust_type) => {
RsTypeKind::ExistingRustType { ref existing_rust_type, .. } => {
let cpp_type = make_cpp_type_from_item(
existing_rust_type.as_ref(),
existing_rust_type.cc_name.as_ref(),
Expand Down
Loading
Loading