Skip to content

Refactor error handling to include names of unsupported types #797

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

Refactor error handling to include names of unsupported types #797

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
383 changes: 143 additions & 240 deletions rs_bindings_from_cc/generate_bindings/database/code_snippet.rs
View file
Open in desktop

Large diffs are not rendered by default.

80 changes: 78 additions & 2 deletions rs_bindings_from_cc/generate_bindings/database/db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ memoized::query_group! {
/// Implementation: rs_bindings_from_cc/generate_bindings/generate_function.rs?q=function:generate_function
fn generate_function(&self, func: Rc<Func>, derived_record: Option<Rc<Record>>) -> Result<Option<GeneratedFunction>>;


/// You should call is_function_ambiguous() instead.
///
/// Identifies all functions having overloads that we can't import (yet).
Expand Down Expand Up @@ -217,7 +216,11 @@ impl<'db> BindingsGenerator<'db> {
match self.type_target_restriction(rs_type_kind.clone())? {
Some(label) if &label != library => {
let rs_type_kind = rs_type_kind.display(self);
Err(anyhow!("{rs_type_kind} is `pub(crate)` in {label}"))
Err(anyhow!(
"crubit.rs/errors/visibility: Support for `{rs_type_kind}` is experimental.\n\
Its use is restricted to `pub(crate)` in defining target:\n\
`{label}`"
))
}
Some(_) => Ok(Visibility::PubCrate),
None => {
Expand Down Expand Up @@ -322,6 +325,79 @@ impl<'db> BindingsGenerator<'db> {
return format! {"{qualifier}{name}"}.into();
}

pub fn cc_type_debug_name(&self, cc_type: &CcType) -> String {
let base_name = match &cc_type.variant {
ir::CcTypeVariant::Primitive(p) => match p {
ir::Primitive::Bool => "bool",
ir::Primitive::Void => "void",
ir::Primitive::Float => "float",
ir::Primitive::Double => "double",
ir::Primitive::Char => "char",
ir::Primitive::SignedChar => "signed char",
ir::Primitive::UnsignedChar => "unsigned char",
ir::Primitive::Short => "short",
ir::Primitive::Int => "int",
ir::Primitive::Long => "long",
ir::Primitive::LongLong => "long long",
ir::Primitive::UnsignedShort => "unsigned short",
ir::Primitive::UnsignedInt => "unsigned int",
ir::Primitive::UnsignedLong => "unsigned long",
ir::Primitive::UnsignedLongLong => "unsigned long long",
ir::Primitive::Char16T => "char16_t",
ir::Primitive::Char32T => "char32_t",
ir::Primitive::PtrdiffT => "ptrdiff_t",
ir::Primitive::IntptrT => "intptr_t",
ir::Primitive::SizeT => "size_t",
ir::Primitive::UintptrT => "uintptr_t",
ir::Primitive::StdPtrdiffT => "std::ptrdiff_t",
ir::Primitive::StdIntptrT => "std::intptr_t",
ir::Primitive::StdSizeT => "std::size_t",
ir::Primitive::StdUintptrT => "std::uintptr_t",
ir::Primitive::Int8T => "int8_t",
ir::Primitive::Int16T => "int16_t",
ir::Primitive::Int32T => "int32_t",
ir::Primitive::Int64T => "int64_t",
ir::Primitive::StdInt8T => "std::int8_t",
ir::Primitive::StdInt16T => "std::int16_t",
ir::Primitive::StdInt32T => "std::int32_t",
ir::Primitive::StdInt64T => "std::int64_t",
ir::Primitive::Uint8T => "uint8_t",
ir::Primitive::Uint16T => "uint16_t",
ir::Primitive::Uint32T => "uint32_t",
ir::Primitive::Uint64T => "uint64_t",
ir::Primitive::StdUint8T => "std::uint8_t",
ir::Primitive::StdUint16T => "std::uint16_t",
ir::Primitive::StdUint32T => "std::uint32_t",
ir::Primitive::StdUint64T => "std::uint64_t",
}
.to_string(),
ir::CcTypeVariant::Pointer(ptr) => {
let ptr_str = match ptr.kind {
ir::PointerTypeKind::LValueRef => "&",
ir::PointerTypeKind::RValueRef => "&&",
ir::PointerTypeKind::Nullable
| ir::PointerTypeKind::NonNull
| ir::PointerTypeKind::Owned => "*",
};
let pointee_name = self.cc_type_debug_name(&ptr.pointee_type);
format!("{pointee_name}{ptr_str}")
}
ir::CcTypeVariant::FuncPointer { .. } => "function pointer".to_string(),
ir::CcTypeVariant::Decl(id) => self.debug_name(*id).to_string(),
ir::CcTypeVariant::Error(err) => format!("<error: {}>", err.message),
};

if cc_type.is_const {
if matches!(cc_type.variant, ir::CcTypeVariant::Pointer(_)) {
format!("{} const", base_name)
} else {
format!("const {}", base_name)
}
} else {
base_name
}
}

pub fn new_unsupported_item(
&self,
item: &impl GenericItem,
Expand Down
99 changes: 52 additions & 47 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 @@ -13,7 +13,6 @@ use crubit_feature::CrubitFeature;
use error_report::{bail, ensure};
use flagset::FlagSet;
use ir::*;
use itertools::Itertools;
use proc_macro2::{Delimiter, Group, Ident, TokenStream, TokenTree};
use quote::{format_ident, quote, ToTokens};
use std::collections::HashSet;
Expand Down Expand Up @@ -325,7 +324,8 @@ impl UniformReprTemplateType {
let arg_type_kind = db.rs_type_kind(template_arg.clone())?;
ensure!(
!arg_type_kind.is_bridge_type(),
"Bridge types cannot be used as template arguments"
"`{}` cannot be used as a template argument because it is a bridged type\nSee crubit.rs/types.",
arg_type_kind.display(db),
);
// We don't do this in required_crubit_features() because it doesn't know which
// template arguments actually need to be free of errors. (For example,
Expand All @@ -338,13 +338,19 @@ impl UniformReprTemplateType {
match template_specialization_kind {
Some(TemplateSpecializationKind::StdUniquePtr { element_type }) => {
let element_type = type_arg(element_type)?;
ensure!(element_type.is_complete(), "Rust std::unique_ptr<T> cannot be used with incomplete types, and `{}` is incomplete", element_type.display(db));
ensure!(element_type.is_destructible(), "Rust std::unique_ptr<T> requires that `T` be destructible, but the destructor of `{}` is non-public or deleted", element_type.display(db));
ensure!(element_type.is_complete(),
"`{}` can't be used in a Rust std::unique_ptr<T> because it is an incomplete type",
element_type.display(db));
ensure!(element_type.is_destructible(),
"`{}` can't be used in a Rust std::unique_ptr<T> because it has a deleted or non-public destructor",
element_type.display(db));
Ok(Some(Rc::new(UniformReprTemplateType::StdUniquePtr { element_type })))
}
Some(TemplateSpecializationKind::StdVector { element_type }) => {
let element_type = type_arg(element_type)?;
ensure!(element_type.is_destructible(), "Rust std::vector<T> requires that `T` be destructible, but the destructor of `{}` is non-public or deleted", element_type.display(db));
ensure!(element_type.is_destructible(),
"`{}` can't be used in a Rust std::vector<T> becuase it has a deleted or non-public destructor",
element_type.display(db));
if element_type.overloads_operator_delete() {
return Ok(None);
}
Expand Down Expand Up @@ -720,7 +726,12 @@ fn new_c9_co_record(
};
let arg_type_kind = db.rs_type_kind(element_type.clone())?;
if let RsTypeKind::Error { error, .. } = arg_type_kind {
return Err(error);
let element_name = db.cc_type_debug_name(element_type);
let desc = error.to_string().replace("\n", "\n ");
return Err(anyhow!(
"`c9::Co<{element_name}>` is unsupported because `{element_name}` is unavailable:\n\
{desc}"
));
};
Ok(Some(BridgeRsTypeKind::C9Co { has_reference_param, result_type: Rc::new(arg_type_kind) }))
}
Expand Down Expand Up @@ -977,59 +988,53 @@ impl RsTypeKind {
/// for both the template definition and its instantiation, and so both
/// would need to be passed in to rs_type_kind() in order to be able to
/// merge these two functions.
pub fn required_crubit_features<'a>(
pub fn missing_feature_descriptions_of_type<'a>(
&self,
target: &BazelLabel,
enabled_features: flagset::FlagSet<CrubitFeature>,
) -> (flagset::FlagSet<CrubitFeature>, String) {
) -> Vec<String> {
let mut missing_features = <flagset::FlagSet<CrubitFeature>>::default();
let mut reasons = <std::collections::BTreeSet<std::borrow::Cow<'static, str>>>::new();
let mut require_feature =
|required_feature: CrubitFeature,
reason: Option<&dyn Fn() -> std::borrow::Cow<'static, str>>| {
let required_features = <flagset::FlagSet<CrubitFeature>>::from(required_feature);
let missing = required_features - enabled_features;
if !missing.is_empty() {
missing_features |= missing;
if let Some(reason) = reason {
reasons.insert(reason());
}
}
};
let mut reasons = std::collections::BTreeSet::<String>::new();
let mut require_feature = |required_feature: CrubitFeature, reason: std::fmt::Arguments| {
let required_features = <flagset::FlagSet<CrubitFeature>>::from(required_feature);
let missing = required_features - enabled_features;
if !missing.is_empty() {
missing_features |= missing;
reasons.insert(reason.to_string());
}
};

if !enabled_features.contains(CrubitFeature::Types) {
require_feature(
CrubitFeature::Types,
format_args!("Crubit is not enabled on defining target:\n {target}"),
);
}

for rs_type_kind in self.dfs_iter() {
match rs_type_kind {
RsTypeKind::Error { error, visibility_override, .. } => {
if visibility_override.is_some() {
require_feature(CrubitFeature::Types, None)
} else {
require_feature(
CrubitFeature::Wrapper,
Some(&|| std::borrow::Cow::from(error.to_string())),
)
if visibility_override.is_none() {
require_feature(CrubitFeature::Wrapper, format_args!("{error}"))
}
}
RsTypeKind::Pointer { .. } => require_feature(CrubitFeature::Types, None),
RsTypeKind::Reference { .. } | RsTypeKind::RvalueReference { .. } => {
require_feature(
CrubitFeature::Experimental,
Some(&|| "references are not yet supported".into()),
format_args!("references are not yet supported"),
);
}
RsTypeKind::FuncPtr { cc_calling_conv, .. } => {
if *cc_calling_conv == CcCallingConv::C {
require_feature(CrubitFeature::Types, None);
} else {
if *cc_calling_conv != CcCallingConv::C {
require_feature(
CrubitFeature::Experimental,
Some(&|| {
"function pointers using a non-`C` calling convention are not yet supported".into()
}),
format_args!("function pointers using a non-`C` calling convention are not yet supported"),
);
}
}
RsTypeKind::IncompleteRecord { .. } => require_feature(
CrubitFeature::Wrapper,
Some(&|| "forward declared types are not yet supported".into()),
format_args!("forward declared types are not yet supported"),
),
// Here, we can very carefully be non-recursive into the _structure_ of the type.
//
Expand All @@ -1046,23 +1051,23 @@ impl RsTypeKind {
// temporary solution until we have a better way to handle template
// instantiations.

if uniform_repr_template_type.is_some() || record.has_unique_owning_target() {
require_feature(CrubitFeature::Types, None);
} else {
if uniform_repr_template_type.is_none() && !record.has_unique_owning_target() {
require_feature(
CrubitFeature::Wrapper,
Some(&|| "template instantiation is not yet supported".into()),
format_args!("template instantiation is not yet supported"),
)
}
}
RsTypeKind::BridgeType { .. } => require_feature(CrubitFeature::Types, None),
RsTypeKind::ExistingRustType { .. }

RsTypeKind::Pointer { .. }
| RsTypeKind::BridgeType { .. }
| RsTypeKind::ExistingRustType { .. }
| RsTypeKind::Enum { .. }
| RsTypeKind::TypeAlias { .. }
| RsTypeKind::Primitive { .. } => require_feature(CrubitFeature::Types, None),
| RsTypeKind::Primitive { .. } => {}
}
}
(missing_features, reasons.into_iter().join(", "))
reasons.into_iter().collect()
}

// If the type is pointer annotated with CRUBIT_OWNED_POINTER, returns the pointee type.
Expand Down Expand Up @@ -1138,11 +1143,11 @@ impl RsTypeKind {
/// it can't.
pub fn check_by_value(&self) -> Result<()> {
match self.unalias() {
RsTypeKind::Error { error, .. } => bail!("Cannot use an error type by value: {error}"),
RsTypeKind::Error { error, .. } => Err(error.clone()),
RsTypeKind::Record { record, .. } => record.check_by_value(),
RsTypeKind::IncompleteRecord { incomplete_record, .. } => {
bail!(
"Attempted to pass incomplete record type `{}` by-value",
"`{}` cannot be passed by-value because it is an incomplete type",
incomplete_record.cc_name
)
}
Expand Down
4 changes: 2 additions & 2 deletions rs_bindings_from_cc/generate_bindings/generate_struct_and_union.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ fn get_field_rs_type_kind_for_layout(
db.defining_target(record.id()).as_ref().into_iter().chain([&record.owning_target])
{
let enabled_features = ir.target_crubit_features(target);
let (missing_features, reason) = type_kind.required_crubit_features(enabled_features);
ensure!(missing_features.is_empty(), reason);
let reasons = type_kind.missing_feature_descriptions_of_type(target, enabled_features);
ensure!(reasons.is_empty(), reasons.join(", "));
}

// In supported, we replace nontrivial fields with opaque blobs.
Expand Down
Loading