Skip to content

Commit

Permalink
Cease to require bases to be allowlisted.
Browse files Browse the repository at this point in the history
Previously we required base classes to be on the allowlist,
on the assumption that we were otherwise unable to see functions
belonging to base classes and thus determine whether they
were abstract.

This was possibly a wrong assumption - it looks like bindgen
fully generates base classes for anything on the allowlist.
Let's test that.

Relates to #774.
  • Loading branch information
adetaylor committed Feb 8, 2022
1 parent 1c57dc9 commit 71382b2
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 18 deletions.
20 changes: 3 additions & 17 deletions engine/src/conversion/analysis/abstract_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,20 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use autocxx_parser::IncludeCppConfig;

use super::{
fun::{FnAnalysis, FnKind, FnPhase, MethodKind, TraitMethodKind},
pod::PodAnalysis,
};
use crate::conversion::api::Api;
use crate::conversion::{
api::TypeKind,
error_reporter::{convert_apis, convert_item_apis},
ConvertError,
};
use crate::{conversion::api::Api, types::QualifiedName};
use std::collections::HashSet;

/// Spot types with pure virtual functions and mark them abstract.
pub(crate) fn mark_types_abstract(
config: &IncludeCppConfig,
mut apis: Vec<Api<FnPhase>>,
) -> Vec<Api<FnPhase>> {
pub(crate) fn mark_types_abstract(mut apis: Vec<Api<FnPhase>>) -> Vec<Api<FnPhase>> {
let mut abstract_types: HashSet<_> = apis
.iter()
.filter_map(|api| match &api {
Expand Down Expand Up @@ -67,10 +62,7 @@ pub(crate) fn mark_types_abstract(
Api::Struct {
analysis: PodAnalysis { bases, kind, .. },
..
} if *kind != TypeKind::Abstract
&& (!abstract_types.is_disjoint(bases)
|| any_missing_from_allowlist(config, bases)) =>
{
} if *kind != TypeKind::Abstract && (!abstract_types.is_disjoint(bases)) => {
*kind = TypeKind::Abstract;
abstract_types.insert(api.name().clone());
// Recurse in case there are further dependent types
Expand Down Expand Up @@ -127,12 +119,6 @@ pub(crate) fn mark_types_abstract(
results
}

fn any_missing_from_allowlist(config: &IncludeCppConfig, bases: &HashSet<QualifiedName>) -> bool {
bases
.iter()
.any(|qn| !config.is_on_allowlist(&qn.to_cpp_name()))
}

pub(crate) fn discard_ignored_functions(apis: Vec<Api<FnPhase>>) -> Vec<Api<FnPhase>> {
// Some APIs can't be generated, e.g. because they're protected.
// Now we've finished analyzing abstract types and constructors, we'll
Expand Down
2 changes: 1 addition & 1 deletion engine/src/conversion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl<'a> BridgeConverter<'a> {
// to generate UniquePtr implementations for the type, since it can't
// be instantiated.
Self::dump_apis_with_deps("analyze fns", &analyzed_apis);
let analyzed_apis = mark_types_abstract(self.config, analyzed_apis);
let analyzed_apis = mark_types_abstract(analyzed_apis);
Self::dump_apis_with_deps("marking abstract", &analyzed_apis);
let analyzed_apis = discard_ignored_functions(analyzed_apis);
Self::dump_apis_with_deps("ignoring ignorable fns", &analyzed_apis);
Expand Down

0 comments on commit 71382b2

Please sign in to comment.