Allow opaque enums (#743)

* opaque enums work, now just need to rename to not be `OpaqueStruct`

* adding OpaqueType instead of OpaqueStruct, and adding tests

* CLIPPy

* fixing rebase

---------

Co-authored-by: Ellen Arteca <emarteca@google.com>

core/src/ast/lifetimes.rs

@@ -155,6 +155,18 @@ impl LifetimeEnv {
         LifetimeEnv::new()
     }
 
+    pub fn from_enum_item(
+        enm: &syn::ItemEnum,
+        variant_fields: &[(Option<Ident>, TypeName, Docs, Attrs)],
+    ) -> Self {
+        let mut this = LifetimeEnv::new();
+        this.extend_generics(&enm.generics);
+        for (_, typ, _, _) in variant_fields {
+            this.extend_implicit_lifetime_bounds(typ, None);
+        }
+        this
+    }
+
     /// Returns a [`LifetimeEnv`] for a struct, accounding for lifetimes and bounds
     /// defined in the struct generics, as well as implicit lifetime bounds in
     /// the struct's fields. For example, the field `&'a Foo<'b>` implies `'b: 'a`.

core/src/ast/mod.rs

@@ -13,7 +13,10 @@ mod modules;
 pub use modules::{File, Module};
 
 mod structs;
-pub use structs::{OpaqueStruct, Struct};
+pub use structs::Struct;
+
+mod opaque;
+pub use opaque::OpaqueType;
 
 mod traits;
 pub use traits::{Trait, TraitMethod};

core/src/ast/modules.rs

@@ -6,8 +6,8 @@ use serde::Serialize;
 use syn::{ImplItem, Item, ItemMod, UseTree, Visibility};
 
 use super::{
-    AttrInheritContext, Attrs, CustomType, Enum, Ident, Method, ModSymbol, Mutability,
-    OpaqueStruct, Path, PathType, RustLink, Struct, Trait,
+    AttrInheritContext, Attrs, CustomType, Enum, Ident, Method, ModSymbol, Mutability, OpaqueType,
+    Path, PathType, RustLink, Struct, Trait,
 };
 use crate::environment::*;
 
@@ -17,13 +17,20 @@ enum DiplomatStructAttribute {
     /// The `#[diplomat::out]` attribute, used for non-opaque structs that
     /// contain an owned opaque in the form of a `Box`.
     Out,
-    /// The `#[diplomat::opaque]` attribute, used for marking a struct as opaque.
+    /// An attribute that can correspond to a type (struct or enum).
+    TypeAttr(DiplomatTypeAttribute),
+}
+
+/// Custom Diplomat attribute that can be placed on an enum or struct definition.
+#[derive(Debug)]
+enum DiplomatTypeAttribute {
+    /// The `#[diplomat::opaque]` attribute, used for marking a type as opaque.
     /// Note that opaque structs can be borrowed in return types, but cannot
     /// be passed into a function behind a mutable reference.
     Opaque,
-    /// The `#[diplomat::opaque_mut]` attribute, used for marking a struct as
+    /// The `#[diplomat::opaque_mut]` attribute, used for marking a type as
     /// opaque and mutable.
-    /// Note that mutable opaque structs can never be borrowed in return types
+    /// Note that mutable opaque types can never be borrowed in return types
     /// (even immutably!), but can be passed into a function behind a mutable
     /// reference.
     OpaqueMut,
@@ -41,6 +48,35 @@ impl DiplomatStructAttribute {
             write!(&mut buf, "{}", attr.path().to_token_stream()).unwrap();
             let parsed = match buf.as_str() {
                 "diplomat :: out" => Some(Self::Out),
+                "diplomat :: opaque" => Some(Self::TypeAttr(DiplomatTypeAttribute::Opaque)),
+                "diplomat :: opaque_mut" => Some(Self::TypeAttr(DiplomatTypeAttribute::OpaqueMut)),
+                _ => None,
+            };
+
+            if let Some(parsed) = parsed {
+                match res {
+                    Ok(None) => res = Ok(Some(parsed)),
+                    Ok(Some(first)) => res = Err(vec![first, parsed]),
+                    Err(ref mut errors) => errors.push(parsed),
+                }
+            }
+        }
+
+        res
+    }
+}
+
+impl DiplomatTypeAttribute {
+    /// Parses a [`DiplomatTypeAttribute`] from an array of [`syn::Attribute`]s.
+    /// If more than one kind is found, an error is returned containing all the
+    /// ones encountered, since all the current attributes are disjoint.
+    fn parse(attrs: &[syn::Attribute]) -> Result<Option<Self>, Vec<Self>> {
+        let mut buf = String::with_capacity(32);
+        let mut res = Ok(None);
+        for attr in attrs {
+            buf.clear();
+            write!(&mut buf, "{}", attr.path().to_token_stream()).unwrap();
+            let parsed = match buf.as_str() {
                 "diplomat :: opaque" => Some(Self::Opaque),
                 "diplomat :: opaque_mut" => Some(Self::OpaqueMut),
                 _ => None,
@@ -155,11 +191,11 @@ impl Module {
                             Ok(Some(DiplomatStructAttribute::Out)) => {
                                 CustomType::Struct(Struct::new(strct, true, &type_parent_attrs))
                             }
-                            Ok(Some(DiplomatStructAttribute::Opaque)) => {
-                                CustomType::Opaque(OpaqueStruct::new(strct, Mutability::Immutable, &type_parent_attrs))
+                            Ok(Some(DiplomatStructAttribute::TypeAttr(DiplomatTypeAttribute::Opaque))) => {
+                                CustomType::Opaque(OpaqueType::new_struct(strct, Mutability::Immutable, &type_parent_attrs))
                             }
-                            Ok(Some(DiplomatStructAttribute::OpaqueMut)) => {
-                                CustomType::Opaque(OpaqueStruct::new(strct, Mutability::Mutable, &type_parent_attrs))
+                            Ok(Some(DiplomatStructAttribute::TypeAttr(DiplomatTypeAttribute::OpaqueMut))) => {
+                                CustomType::Opaque(OpaqueType::new_struct(strct, Mutability::Mutable, &type_parent_attrs))
                             }
                             Err(errors) => {
                                 panic!("Multiple conflicting Diplomat struct attributes, there can be at most one: {errors:?}");
@@ -173,9 +209,20 @@ impl Module {
                 Item::Enum(enm) => {
                     if analyze_types {
                         let ident = (&enm.ident).into();
-                        let enm = Enum::new(enm, &type_parent_attrs);
+                        let custom_enum = match DiplomatTypeAttribute::parse(&enm.attrs[..]) {
+                            Ok(None) => CustomType::Enum(Enum::new(enm, &type_parent_attrs)),
+                            Ok(Some(DiplomatTypeAttribute::Opaque)) => {
+                                CustomType::Opaque(OpaqueType::new_enum(enm, Mutability::Immutable, &type_parent_attrs))
+                            }
+                            Ok(Some(DiplomatTypeAttribute::OpaqueMut)) => {
+                                CustomType::Opaque(OpaqueType::new_enum(enm, Mutability::Mutable, &type_parent_attrs))
+                            }
+                            Err(errors) => {
+                                panic!("Multiple conflicting Diplomat enum attributes, there can be at most one: {errors:?}");
+                            }
+                        };
                         custom_types_by_name
-                            .insert(ident, CustomType::Enum(enm));
+                            .insert(ident, custom_enum);
                     }
                 }
 

core/src/ast/opaque.rs

@@ -0,0 +1,64 @@
+use serde::Serialize;
+
+use super::docs::Docs;
+use super::{Attrs, Ident, LifetimeEnv, Method, Mutability};
+
+/// A type annotated with [`diplomat::opaque`] whose fields/variants are not visible.
+/// Opaque types cannot be passed by-value across the FFI boundary, so they
+/// must be boxed or passed as references.
+#[derive(Clone, Serialize, Debug, Hash, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct OpaqueType {
+    pub name: Ident,
+    pub docs: Docs,
+    pub lifetimes: LifetimeEnv,
+    pub methods: Vec<Method>,
+    pub mutability: Mutability,
+    pub attrs: Attrs,
+    /// The ABI name of the generated destructor
+    pub dtor_abi_name: Ident,
+}
+
+impl OpaqueType {
+    /// Extract a [`OpaqueType`] metadata value from an AST node representing a struct.
+    pub fn new_struct(
+        strct: &syn::ItemStruct,
+        mutability: Mutability,
+        parent_attrs: &Attrs,
+    ) -> Self {
+        let mut attrs = parent_attrs.clone();
+        attrs.add_attrs(&strct.attrs);
+        let name = Ident::from(&strct.ident);
+        OpaqueType {
+            dtor_abi_name: Self::dtor_abi_name(&name, &attrs),
+            name,
+            docs: Docs::from_attrs(&strct.attrs),
+            lifetimes: LifetimeEnv::from_struct_item(strct, &[]),
+            methods: vec![],
+            mutability,
+            attrs,
+        }
+    }
+
+    /// Extract a [`OpaqueType`] metadata value from an AST node representing an enum.
+    pub fn new_enum(enm: &syn::ItemEnum, mutability: Mutability, parent_attrs: &Attrs) -> Self {
+        let mut attrs = parent_attrs.clone();
+        attrs.add_attrs(&enm.attrs);
+        let name = Ident::from(&enm.ident);
+        OpaqueType {
+            dtor_abi_name: Self::dtor_abi_name(&name, &attrs),
+            name,
+            docs: Docs::from_attrs(&enm.attrs),
+            lifetimes: LifetimeEnv::from_enum_item(enm, &[]),
+            methods: vec![],
+            mutability,
+            attrs,
+        }
+    }
+
+    fn dtor_abi_name(name: &Ident, attrs: &Attrs) -> Ident {
+        let dtor_abi_name = format!("{}_destroy", name);
+        let dtor_abi_name = String::from(attrs.abi_rename.apply(dtor_abi_name.into()));
+        Ident::from(dtor_abi_name)
+    }
+}

core/src/ast/structs.rs

@@ -1,7 +1,7 @@
 use serde::Serialize;
 
 use super::docs::Docs;
-use super::{Attrs, Ident, LifetimeEnv, Method, Mutability, PathType, TypeName};
+use super::{Attrs, Ident, LifetimeEnv, Method, PathType, TypeName};
 
 /// A struct declaration in an FFI module that is not opaque.
 #[derive(Clone, PartialEq, Eq, Hash, Serialize, Debug)]
@@ -52,43 +52,6 @@ impl Struct {
     }
 }
 
-/// A struct annotated with [`diplomat::opaque`] whose fields are not visible.
-/// Opaque structs cannot be passed by-value across the FFI boundary, so they
-/// must be boxed or passed as references.
-#[derive(Clone, Serialize, Debug, Hash, PartialEq, Eq)]
-#[non_exhaustive]
-pub struct OpaqueStruct {
-    pub name: Ident,
-    pub docs: Docs,
-    pub lifetimes: LifetimeEnv,
-    pub methods: Vec<Method>,
-    pub mutability: Mutability,
-    pub attrs: Attrs,
-    /// The ABI name of the generated destructor
-    pub dtor_abi_name: Ident,
-}
-
-impl OpaqueStruct {
-    /// Extract a [`OpaqueStruct`] metadata value from an AST node.
-    pub fn new(strct: &syn::ItemStruct, mutability: Mutability, parent_attrs: &Attrs) -> Self {
-        let mut attrs = parent_attrs.clone();
-        attrs.add_attrs(&strct.attrs);
-        let name = Ident::from(&strct.ident);
-        let dtor_abi_name = format!("{}_destroy", name);
-        let dtor_abi_name = String::from(attrs.abi_rename.apply(dtor_abi_name.into()));
-        let dtor_abi_name = Ident::from(dtor_abi_name);
-        OpaqueStruct {
-            name,
-            docs: Docs::from_attrs(&strct.attrs),
-            lifetimes: LifetimeEnv::from_struct_item(strct, &[]),
-            methods: vec![],
-            mutability,
-            attrs,
-            dtor_abi_name,
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use insta::{self, Settings};

core/src/ast/types.rs

@@ -9,7 +9,7 @@ use std::str::FromStr;
 
 use super::{
     Attrs, Docs, Enum, Ident, Lifetime, LifetimeEnv, LifetimeTransitivity, Method, NamedLifetime,
-    OpaqueStruct, Path, RustLink, Struct, Trait,
+    OpaqueType, Path, RustLink, Struct, Trait,
 };
 use crate::Env;
 
@@ -19,8 +19,8 @@ use crate::Env;
 pub enum CustomType {
     /// A non-opaque struct whose fields will be visible across the FFI boundary.
     Struct(Struct),
-    /// A struct annotated with [`diplomat::opaque`] whose fields are not visible.
-    Opaque(OpaqueStruct),
+    /// A type annotated with [`diplomat::opaque`] whose fields are not visible.
+    Opaque(OpaqueType),
     /// A fieldless enum.
     Enum(Enum),
 }

core/src/hir/lowering.rs

@@ -179,7 +179,7 @@ impl<'ast> LoweringContext<'ast> {
     }
     pub(super) fn lower_all_opaques(
         &mut self,
-        ast_defs: impl ExactSizeIterator<Item = ItemAndInfo<'ast, ast::OpaqueStruct>>,
+        ast_defs: impl ExactSizeIterator<Item = ItemAndInfo<'ast, ast::OpaqueType>>,
     ) -> Result<Vec<OpaqueDef>, ()> {
         self.lower_all(ast_defs, Self::lower_opaque)
     }
@@ -257,10 +257,7 @@ impl<'ast> LoweringContext<'ast> {
         Ok(def)
     }
 
-    fn lower_opaque(
-        &mut self,
-        item: ItemAndInfo<'ast, ast::OpaqueStruct>,
-    ) -> Result<OpaqueDef, ()> {
+    fn lower_opaque(&mut self, item: ItemAndInfo<'ast, ast::OpaqueType>) -> Result<OpaqueDef, ()> {
         let ast_opaque = item.item;
         self.errors.set_item(ast_opaque.name.as_str());
         let name = self.lower_ident(&ast_opaque.name, "opaque name");

core/src/hir/snapshots/diplomat_core__hir__type_context__tests__opaque_ffi.snap

@@ -1,7 +1,9 @@
 ---
 source: core/src/hir/type_context.rs
+assertion_line: 691
 expression: output
 ---
+Lowering error in MyOpaqueEnum::new_broken: Opaque passed by value in input: MyOpaqueEnum
+Lowering error in MyOpaqueEnum::do_thing_broken: Method `MyOpaqueEnum_do_thing_broken` takes an opaque by value as the self parameter, but opaques as inputs must be behind refs
 Lowering error in MyOpaqueStruct::new_broken: Opaque passed by value in input: MyOpaqueStruct
 Lowering error in MyOpaqueStruct::do_thing_broken: Method `MyOpaqueStruct_do_thing_broken` takes an opaque by value as the self parameter, but opaques as inputs must be behind refs
-

core/src/hir/type_context.rs

@@ -459,7 +459,7 @@ impl TypeContext {
 pub(super) struct LookupId<'ast> {
     out_struct_map: HashMap<&'ast ast::Struct, OutStructId>,
     struct_map: HashMap<&'ast ast::Struct, StructId>,
-    opaque_map: HashMap<&'ast ast::OpaqueStruct, OpaqueId>,
+    opaque_map: HashMap<&'ast ast::OpaqueType, OpaqueId>,
     enum_map: HashMap<&'ast ast::Enum, EnumId>,
     trait_map: HashMap<&'ast ast::Trait, TraitId>,
 }
@@ -469,7 +469,7 @@ impl<'ast> LookupId<'ast> {
     fn new(
         out_structs: &[ItemAndInfo<'ast, ast::Struct>],
         structs: &[ItemAndInfo<'ast, ast::Struct>],
-        opaques: &[ItemAndInfo<'ast, ast::OpaqueStruct>],
+        opaques: &[ItemAndInfo<'ast, ast::OpaqueType>],
         enums: &[ItemAndInfo<'ast, ast::Enum>],
         traits: &[ItemAndInfo<'ast, ast::Trait>],
     ) -> Self {
@@ -510,7 +510,7 @@ impl<'ast> LookupId<'ast> {
         self.struct_map.get(strct).copied()
     }
 
-    pub(super) fn resolve_opaque(&self, opaque: &ast::OpaqueStruct) -> Option<OpaqueId> {
+    pub(super) fn resolve_opaque(&self, opaque: &ast::OpaqueType) -> Option<OpaqueId> {
         self.opaque_map.get(opaque).copied()
     }
 
@@ -701,6 +701,21 @@ mod tests {
                     pub fn do_thing_broken(self) {}
                     pub fn broken_differently(&self, x: &MyOpaqueStruct) {}
                 }
+
+                #[diplomat::opaque]
+                enum MyOpaqueEnum {
+                    A(UnknownType),
+                    B,
+                    C(i32, i32, UnknownType2),
+                }
+
+                impl MyOpaqueEnum {
+                    pub fn new() -> Box<MyOpaqueEnum> {}
+                    pub fn new_broken() -> MyOpaqueEnum {}
+                    pub fn do_thing(&self) {}
+                    pub fn do_thing_broken(self) {}
+                    pub fn broken_differently(&self, x: &MyOpaqueEnum) {}
+                }
             }
         }
     }