Pure virtuals

engine/Cargo.toml

@@ -32,7 +32,7 @@ proc-macro2 = "1.0.11"
 quote = "1.0"
 lazy_static = "1.4"
 indoc = "1.0"
-autocxx-bindgen = "0.57.2"
+autocxx-bindgen = "0.57.3"
 itertools = "0.9"
 cc = { version = "1.0", optional = true }
 unzip-n = "0.1.2"

engine/src/conversion/analysis/abstract_types.rs

@@ -0,0 +1,50 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use super::fun::{FnAnalysis, FnAnalysisBody};
+use crate::conversion::api::ApiDetail;
+use crate::conversion::api::{Api, TypeKind};
+use std::collections::HashSet;
+
+/// Spot types with pure virtual functions and mark them abstract.
+pub(crate) fn mark_types_abstract(apis: &mut Vec<Api<FnAnalysis>>) {
+    let abstract_types: HashSet<_> = apis
+        .iter()
+        .filter_map(|api| match &api.detail {
+            ApiDetail::Function {
+                fun: _,
+                analysis:
+                    FnAnalysisBody {
+                        is_pure_virtual: true,
+                        self_ty: Some(self_ty_name),
+                        ..
+                    },
+            } => Some(self_ty_name.clone()),
+            _ => None,
+        })
+        .collect();
+    if abstract_types.is_empty() {
+        return;
+    }
+
+    for api in apis {
+        let tyname = api.typename();
+        match &mut api.detail {
+            ApiDetail::Type { analysis, .. } if abstract_types.contains(&tyname) => {
+                *analysis = TypeKind::Abstract;
+            }
+            _ => {}
+        }
+    }
+}

engine/src/conversion/analysis/fun/mod.rs

@@ -20,6 +20,7 @@ use crate::known_types::KNOWN_TYPES;
 use std::collections::{HashMap, HashSet};
 
 use autocxx_parser::{TypeConfig, UnsafePolicy};
+use proc_macro2::Span;
 use syn::{
     parse_quote, punctuated::Punctuated, FnArg, ForeignItemFn, Ident, LitStr, Pat, ReturnType,
     Type, TypePtr, Visibility,
@@ -61,6 +62,7 @@ pub(crate) struct FnAnalysisBody {
     pub(crate) id_for_allowlist: Option<Ident>,
     pub(crate) use_stmt: Use,
     pub(crate) additional_cpp: Option<AdditionalNeed>,
+    pub(crate) is_pure_virtual: bool,
 }
 
 pub(crate) struct ArgumentAnalysis {
@@ -69,7 +71,7 @@ pub(crate) struct ArgumentAnalysis {
     pub(crate) self_type: Option<TypeName>,
     was_reference: bool,
     deps: HashSet<TypeName>,
-    virtual_this_encountered: bool,
+    is_virtual: bool,
     requires_unsafe: bool,
 }
 
@@ -304,6 +306,7 @@ impl<'a> FnAnalyzer<'a> {
     ) -> Result<Option<FnAnalysisResult>, ConvertError> {
         let fun = &func_information.item;
         let virtual_this = &func_information.virtual_this_type;
+        let is_pure_virtual = Self::has_attr(&fun, "bindgen_pure_virtual");
         // This function is one of the most complex parts of our conversion.
         // It needs to consider:
         // 1. Rejecting destructors entirely.
@@ -356,7 +359,10 @@ impl<'a> FnAnalyzer<'a> {
             .filter_map(|pd| pd.self_type.as_ref())
             .next()
             .cloned();
-        let virtual_this_encountered = param_details.iter().any(|pd| pd.virtual_this_encountered);
+        let is_virtual = param_details.iter().any(|pd| pd.is_virtual);
+        if is_pure_virtual {
+            assert!(is_virtual);
+        }
         let requires_unsafe = param_details.iter().any(|pd| pd.requires_unsafe);
 
         let is_static_method = if self_ty.is_none() {
@@ -484,7 +490,7 @@ impl<'a> FnAnalyzer<'a> {
             || ret_type_conversion_needed
             || is_static_method
             || differently_named_method
-            || virtual_this_encountered;
+            || is_virtual;
 
         let mut additional_cpp = None;
 
@@ -602,6 +608,7 @@ impl<'a> FnAnalyzer<'a> {
                 id_for_allowlist,
                 use_stmt,
                 additional_cpp,
+                is_pure_virtual,
             },
             id,
             deps,
@@ -624,7 +631,7 @@ impl<'a> FnAnalyzer<'a> {
                 let mut pt = pt.clone();
                 let mut self_type = None;
                 let old_pat = *pt.pat;
-                let mut virtual_this_encountered = false;
+                let mut is_virtual = false;
                 let mut treat_as_reference = false;
                 let new_pat = match old_pat {
                     syn::Pat::Ident(mut pp) if pp.ident == "this" => {
@@ -635,16 +642,21 @@ impl<'a> FnAnalyzer<'a> {
                                 Type::Path(typ) => {
                                     let mut this_type = TypeName::from_type_path(typ);
                                     if this_type.is_cvoid() {
-                                        virtual_this_encountered = true;
+                                        is_virtual = true;
                                         this_type = virtual_this.ok_or_else(|| {
                                             ConvertError::VirtualThisType(
                                                 ns.clone(),
                                                 fn_name.into(),
                                             )
                                         })?;
                                         let this_type_path = this_type.to_type_path();
+                                        let const_token = if mutability.is_some() {
+                                            None
+                                        } else {
+                                            Some(syn::Token![const](Span::call_site()))
+                                        };
                                         pt.ty = Box::new(parse_quote! {
-                                            * #mutability #this_type_path
+                                            * #mutability #const_token #this_type_path
                                         });
                                     }
                                     Ok(this_type)
@@ -681,7 +693,7 @@ impl<'a> FnAnalyzer<'a> {
                         conversion,
                         was_reference,
                         deps,
-                        virtual_this_encountered,
+                        is_virtual,
                         requires_unsafe,
                     },
                 )
@@ -776,6 +788,10 @@ impl<'a> FnAnalyzer<'a> {
         }
         (ref_params, ref_return)
     }
+
+    fn has_attr(fun: &ForeignItemFn, attr_name: &str) -> bool {
+        fun.attrs.iter().any(|at| at.path.is_ident(attr_name))
+    }
 }
 
 impl Api<FnAnalysis> {

engine/src/conversion/analysis/mod.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+pub(crate) mod abstract_types;
 pub(crate) mod ctypes;
 pub(crate) mod fun;
 pub(crate) mod gc;

engine/src/conversion/api.rs

@@ -24,6 +24,16 @@ pub(crate) enum TypeKind {
     Pod,                // trivial. Can be moved and copied in Rust.
     NonPod, // has destructor or non-trivial move constructors. Can only hold by UniquePtr
     ForwardDeclaration, // no full C++ declaration available - can't even generate UniquePtr
+    Abstract, // has pure virtual members - can't even generate UniquePtr
+}
+
+impl TypeKind {
+    pub(crate) fn can_be_instantiated(&self) -> bool {
+        match self {
+            TypeKind::Pod | TypeKind::NonPod => true,
+            TypeKind::ForwardDeclaration | TypeKind::Abstract => false,
+        }
+    }
 }
 
 /// Whether and how this type should be exposed in the mods constructed

engine/src/conversion/codegen_rs/mod.rs

@@ -348,9 +348,10 @@ impl<'a> RsCodeGenerator<'a> {
             } => RsCodegenResult {
                 global_items: Self::generate_extern_type_impl(analysis, &ty_details),
                 impl_entry: None,
-                bridge_items: match analysis {
-                    TypeKind::ForwardDeclaration => Vec::new(),
-                    _ => create_impl_items(&ty_details.final_ident),
+                bridge_items: if analysis.can_be_instantiated() {
+                    create_impl_items(&ty_details.final_ident)
+                } else {
+                    Vec::new()
                 },
                 extern_c_mod_item: Some(ForeignItem::Verbatim(for_extern_c_ts)),
                 bindgen_mod_item,

engine/src/conversion/mod.rs

@@ -34,8 +34,8 @@ use crate::UnsafePolicy;
 
 use self::{
     analysis::{
-        gc::filter_apis_by_following_edges_from_allowlist, pod::analyze_pod_apis,
-        remove_ignored::filter_apis_by_ignored_dependents,
+        abstract_types::mark_types_abstract, gc::filter_apis_by_following_edges_from_allowlist,
+        pod::analyze_pod_apis, remove_ignored::filter_apis_by_ignored_dependents,
     },
     codegen_rs::RsCodeGenerator,
     parse::ParseBindgen,
@@ -110,12 +110,16 @@ impl<'a> BridgeConverter<'a> {
                 // require C++ wrapper functions. This is probably the most complex
                 // part of `autocxx`. Again, this returns a new set of `Api`s, but
                 // parameterized by a richer set of metadata.
-                let analyzed_apis = FnAnalyzer::analyze_functions(
+                let mut analyzed_apis = FnAnalyzer::analyze_functions(
                     analyzed_apis,
                     unsafe_policy,
                     &mut type_converter,
                     self.type_config,
                 )?;
+                // If any of those functions turned out to be pure virtual, don't attempt
+                // to generate UniquePtr implementations for the type, since it can't
+                // be instantiated.
+                mark_types_abstract(&mut analyzed_apis);
                 // During parsing or subsequent processing we might have encountered
                 // items which we couldn't process due to as-yet-unsupported features.
                 // There might be other items depending on such things. Let's remove them

engine/src/integration_tests.rs

@@ -3696,7 +3696,6 @@ fn test_virtual_fns() {
 }
 
 #[test]
-#[ignore] // https://github.com/google/autocxx/issues/313
 fn test_const_virtual_fns() {
     let hdr = indoc! {"
         #include <cstdint>
@@ -4187,7 +4186,6 @@ fn test_unexpected_use() {
 }
 
 #[test]
-#[ignore] // https://github.com/google/autocxx/issues/305
 fn test_get_pure_virtual() {
     let hdr = indoc! {"
         #include <cstdint>
@@ -4207,7 +4205,7 @@ fn test_get_pure_virtual() {
         let a_ref = unsafe { a.as_ref() }.unwrap();
         assert_eq!(a_ref.get_val(), 3);
     };
-    run_test("", hdr, rs, &["get_a"], &[]);
+    run_test("", hdr, rs, &["A", "get_a"], &[]);
 }
 
 #[test]