Merge pull request #44 from joshua-maros/fix-covariance

Fix covariance detection

examples/src/lib.rs

@@ -30,7 +30,6 @@ pub struct DocumentationExample {
     #[borrows(int_data)]
     int_reference: &'this i32,
     #[borrows(mut float_data)]
-    #[not_covariant]
     float_reference: &'this mut f32,
 }
 

examples/src/ok_tests.rs

@@ -7,7 +7,6 @@ use std::fmt::Debug;
 struct TraitObject {
     data: Box<dyn Debug>,
     #[borrows(data)]
-    #[covariant]
     dref: &'this dyn Debug,
 }
 
@@ -22,7 +21,6 @@ struct BoxAndRef {
 struct BoxAndMutRef {
     data: i32,
     #[borrows(mut data)]
-    #[not_covariant]
     dref: &'this mut i32,
 }
 
@@ -39,7 +37,6 @@ struct ChainedAndUndocumented {
 struct BoxCheckWithLifetimeParameter<'t> {
     external_data: &'t (),
     #[borrows(external_data)]
-    #[covariant]
     self_reference: &'this &'t (),
 }
 
@@ -67,7 +64,6 @@ where
     data2: &'this C,
     data3: B,
     #[borrows(mut data3)]
-    #[not_covariant]
     data4: &'this mut C,
 }
 

ouroboros_macro/src/covariance_detection.rs

@@ -40,88 +40,99 @@ pub fn apparent_std_container_type(raw_type: &Type) -> Option<(&'static str, &Ty
     None
 }
 
-/// Returns true if the specified type can be assumed to be covariant.
-pub fn type_is_covariant(ty: &syn::Type, in_template: bool) -> bool {
+/// Returns Some(true or false) if the type is known to be covariant / not covariant.
+pub fn type_is_covariant_over_this_lifetime(ty: &syn::Type) -> Option<bool> {
     use syn::Type::*;
     // If the type never uses the 'this lifetime, we don't have to
     // worry about it not being covariant.
     if !uses_this_lifetime(ty.to_token_stream()) {
-        return true;
+        return Some(true);
     }
     match ty {
-        Array(arr) => type_is_covariant(&*arr.elem, in_template),
+        Array(arr) => type_is_covariant_over_this_lifetime(&*arr.elem),
         BareFn(f) => {
             for arg in f.inputs.iter() {
-                if !type_is_covariant(&arg.ty, true) {
-                    return false;
+                if uses_this_lifetime(arg.ty.to_token_stream()) {
+                    return None;
                 }
             }
             if let syn::ReturnType::Type(_, ty) = &f.output {
-                type_is_covariant(ty, true)
-            } else {
-                true
+                if uses_this_lifetime(ty.to_token_stream()) {
+                    return None;
+                }
             }
+            Some(true)
         }
-        Group(ty) => type_is_covariant(&ty.elem, in_template),
-        ImplTrait(..) => false, // Unusable in struct definition.
-        Infer(..) => false,     // Unusable in struct definition.
-        Macro(..) => false,     // Assume false since we don't know.
-        Never(..) => false,
-        Paren(ty) => type_is_covariant(&ty.elem, in_template),
+        Group(ty) => type_is_covariant_over_this_lifetime(&ty.elem),
+        ImplTrait(..) => None, // Unusable in struct definition.
+        Infer(..) => None,     // Unusable in struct definition.
+        Macro(..) => None,     // Assume false since we don't know.
+        Never(..) => None,
+        Paren(ty) => type_is_covariant_over_this_lifetime(&ty.elem),
         Path(path) => {
             if let Some(qself) = &path.qself {
-                if !type_is_covariant(&qself.ty, in_template) {
-                    return false;
+                if !type_is_covariant_over_this_lifetime(&qself.ty)? {
+                    return Some(false);
                 }
             }
-            let mut is_covariant = false;
+            let mut all_parameters_are_covariant = false;
             // If the type is Box, Arc, or Rc, we can assume it to be covariant.
             if apparent_std_container_type(ty).is_some() {
-                is_covariant = true;
+                all_parameters_are_covariant = true;
             }
             for segment in path.path.segments.iter() {
                 let args = &segment.arguments;
                 if let syn::PathArguments::AngleBracketed(args) = &args {
                     for arg in args.args.iter() {
                         if let syn::GenericArgument::Type(ty) = arg {
-                            if !type_is_covariant(ty, !is_covariant) {
-                                return false;
+                            if all_parameters_are_covariant {
+                                if !type_is_covariant_over_this_lifetime(ty)? {
+                                    return Some(false);
+                                }
+                            } else {
+                                if uses_this_lifetime(ty.to_token_stream()) {
+                                    return None;
+                                }
                             }
                         } else if let syn::GenericArgument::Lifetime(lt) = arg {
-                            if lt.ident.to_string() == "this" && !is_covariant {
-                                return false;
+                            if lt.ident.to_string() == "this" && !all_parameters_are_covariant {
+                                return None;
                             }
                         }
                     }
                 } else if let syn::PathArguments::Parenthesized(args) = &args {
                     for arg in args.inputs.iter() {
-                        if !type_is_covariant(arg, true) {
-                            return false;
+                        if uses_this_lifetime(arg.to_token_stream()) {
+                            return None;
                         }
                     }
                     if let syn::ReturnType::Type(_, ty) = &args.output {
-                        if !type_is_covariant(ty, true) {
-                            return false;
+                        if uses_this_lifetime(ty.to_token_stream()) {
+                            return None;
                         }
                     }
                 }
             }
-            true
+            Some(true)
         }
-        Ptr(ptr) => type_is_covariant(&ptr.elem, in_template),
+        Ptr(ptr) => type_is_covariant_over_this_lifetime(&ptr.elem),
         // Ignore the actual lifetime of the reference because Rust can automatically convert those.
         Reference(rf) => {
-            !in_template && rf.mutability.is_none() && type_is_covariant(&rf.elem, in_template)
+            if rf.mutability.is_some() {
+                Some(!uses_this_lifetime(rf.elem.clone().into_token_stream()))
+            } else {
+                type_is_covariant_over_this_lifetime(&rf.elem)
+            }
         }
-        Slice(sl) => type_is_covariant(&sl.elem, in_template),
-        TraitObject(..) => false,
+        Slice(sl) => type_is_covariant_over_this_lifetime(&sl.elem),
+        TraitObject(..) => None,
         Tuple(tup) => {
             for ty in tup.elems.iter() {
-                if !type_is_covariant(ty, in_template) {
-                    return false;
+                if !type_is_covariant_over_this_lifetime(ty)? {
+                    return Some(false)
                 }
             }
-            false
+            Some(true)
         }
         // As of writing this, syn parses all the types we could need.
         Verbatim(..) => unimplemented!(),

ouroboros_macro/src/parse.rs

@@ -3,7 +3,7 @@ use quote::format_ident;
 use syn::{spanned::Spanned, Attribute, Error, Fields, GenericParam, ItemStruct};
 
 use crate::{
-    covariance_detection::type_is_covariant,
+    covariance_detection::type_is_covariant_over_this_lifetime,
     info_structures::{BorrowRequest, Derive, FieldType, StructFieldInfo, StructInfo},
     utils::submodule_contents_visiblity,
 };
@@ -141,8 +141,7 @@ pub fn parse_struct(def: &ItemStruct) -> Result<StructInfo, Error> {
             for field in &mut def_fields.named {
                 let mut borrows = Vec::new();
                 let mut self_referencing = false;
-                let covariant = type_is_covariant(&field.ty, false);
-                let mut covariant = if covariant { Some(true) } else { None };
+                let mut covariant = type_is_covariant_over_this_lifetime(&field.ty);
                 let mut remove_attrs = Vec::new();
                 for (index, attr) in field.attrs.iter().enumerate() {
                     let path = &attr.path;
@@ -161,10 +160,16 @@ pub fn parse_struct(def: &ItemStruct) -> Result<StructInfo, Error> {
                         remove_attrs.push(index);
                     }
                     if path.segments.first().unwrap().ident == "covariant" {
+                        if covariant.is_some() {
+                            panic!("TODO: Nice error, covariance specified twice.");
+                        }
                         covariant = Some(true);
                         remove_attrs.push(index);
                     }
                     if path.segments.first().unwrap().ident == "not_covariant" {
+                        if covariant.is_some() {
+                            panic!("TODO: Nice error, covariance specified twice.");
+                        }
                         covariant = Some(false);
                         remove_attrs.push(index);
                     }