Skip to content
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.

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

Added target_os cfg feature to header translator #433

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
107 changes: 95 additions & 12 deletions crates/header-translator/src/availability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,90 @@ use clang::{Entity, PlatformAvailability, Version};
use crate::context::Context;

#[derive(Debug, Clone, PartialEq, Default)]
struct Unavailable {
ios: bool,
ios_app_extension: bool,
macos: bool,
macos_app_extension: bool,
maccatalyst: bool,
watchos: bool,
tvos: bool,
pub struct Unavailable {
pub(crate) ios: bool,
pub(crate) ios_app_extension: bool,
pub(crate) macos: bool,
pub(crate) macos_app_extension: bool,
pub(crate) maccatalyst: bool,
pub(crate) watchos: bool,
pub(crate) tvos: bool,
pub(crate) library_unavailablility: Option<Box<Unavailable>>,
}

impl Unavailable {
fn list_unavailable_oses(&self) -> Vec<&str> {
let mut unavailable_oses = Vec::new();
if self.ios
&& !self
.library_unavailablility
.as_ref()
.map(|u| u.ios)
.unwrap_or_else(|| false)
{
unavailable_oses.push("ios");
}
if self.macos
&& !self
.library_unavailablility
.as_ref()
.map(|u| u.macos)
.unwrap_or_else(|| false)
{
unavailable_oses.push("macos");
}
if self.tvos
&& !self
.library_unavailablility
.as_ref()
.map(|u| u.tvos)
.unwrap_or_else(|| false)
{
unavailable_oses.push("tvos");
}
if self.watchos
&& !self
.library_unavailablility
.as_ref()
.map(|u| u.watchos)
.unwrap_or_else(|| false)
{
unavailable_oses.push("watchos");
}
unavailable_oses
}

/// In some cases of enums, we need to know the availability of the parent enum and the enum
/// variant.
pub fn merge(&self, other: &Self) -> Self {
Self {
ios: self.ios || other.ios,
ios_app_extension: self.ios_app_extension || other.ios_app_extension,
macos: self.macos || other.macos,
macos_app_extension: self.macos_app_extension || other.macos_app_extension,
tvos: self.tvos || other.tvos,
watchos: self.watchos || other.watchos,
maccatalyst: self.maccatalyst || other.maccatalyst,
library_unavailablility: self.library_unavailablility.clone(),
}
}
}
impl fmt::Display for Unavailable {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let unavailable_oses = self.list_unavailable_oses();
let unavailable_oses = unavailable_oses
.iter()
.map(|os| format!("target_os = \"{os}\""))
.collect::<Vec<String>>()
.join(",");
if unavailable_oses.len() > 1 {
write!(f, "#[cfg(not(any({unavailable_oses})))]")?;
simlay marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Owner

@madsmtm madsmtm Apr 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that the headers expose this as "unavailability", but I think it would look nicer in the docs if it was inverted; e.g. that #[cfg(not(any(target_os = "watchos")))] became #[cfg(any(target_os = "tvos", target_os = "ios", target_os = "macos"))] instead (this is also how Apple's own docs does it).

This interferes with GNUStep of course, but perhaps we can just code in a manual override there for all of AppKit and Foundation for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually spent a little bit of time doing both variants - all(not(target_os = "ios"), any(taget_os = "macos", target_os = "tvos", target_os = "watchos")). I didn't like it as then everything had a cfg attribute.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree with that part

}
if unavailable_oses.len() == 1 {
write!(f, "#[cfg(not({unavailable_oses}))]")?;
}
Ok(())
}
}

#[derive(Debug, Clone, PartialEq, Default)]
Expand All @@ -29,20 +105,26 @@ struct Versions {

#[derive(Debug, Clone, PartialEq)]
pub struct Availability {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[deprecated] now also appears on impl ClassType, which is incorrect.

So perhaps we should split out the Unavailable from Availability (and maybe rename it to PlatformCfg)? Since that part is always going to be a cfg-gate, which has interactions with higher-level cfg-gates (and must propagate to imports like generated/[Framework]/mod.rs), while introduced and deprecated will always only need to apply to the statement/method/field they're associated with.

Maybe it would even make sense to add PlatformCfg as a field of ItemIdentifier? Since these two are access so often together.

unavailable: Unavailable,
pub(crate) unavailable: Unavailable,
introduced: Versions,
deprecated: Versions,
message: Option<String>,
_swift: Option<PlatformAvailability>,
}

impl Availability {
pub fn parse(entity: &Entity<'_>, _context: &Context<'_>) -> Self {
pub fn parse(entity: &Entity<'_>, context: &Context<'_>) -> Self {
let availabilities = entity
.get_platform_availability()
.expect("platform availability");

let mut unavailable = Unavailable::default();
let mut unavailable = Unavailable {
library_unavailablility: context
.library_unavailability
.as_ref()
.map(|l| Box::new(l.clone())),
..Default::default()
};
let mut introduced = Versions::default();
let mut deprecated = Versions::default();
let mut message = None;
Expand Down Expand Up @@ -157,7 +239,8 @@ impl fmt::Display for Availability {
}
}
}
// TODO: Emit `cfg` attributes based on `self.unavailable`
write!(f, "{}", self.unavailable)?;

// TODO: Emit availability checks based on `self.introduced`
Ok(())
}
Expand Down
13 changes: 13 additions & 0 deletions crates/header-translator/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::Path;

use serde::Deserialize;

use crate::availability::Unavailable;
use crate::data;
use crate::stmt::{Derives, Mutability};

Expand Down Expand Up @@ -80,6 +81,18 @@ pub struct LibraryData {
#[serde(default)]
pub watchos: Option<semver::VersionReq>,
}
impl LibraryData {
pub(crate) fn unavailability(&self) -> Unavailable {
Unavailable {
ios: self.ios.is_none(),
macos: self.macos.is_none(),
tvos: self.tvos.is_none(),
watchos: self.watchos.is_none(),
maccatalyst: self.maccatalyst.is_none(),
..Default::default()
}
}
}

#[derive(Deserialize, Debug, Default, Clone, PartialEq, Eq)]
#[serde(deny_unknown_fields)]
Expand Down
3 changes: 3 additions & 0 deletions crates/header-translator/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use apple_sdk::SdkPath;
use clang::source::Location;
use clang::Entity;

use crate::availability::Unavailable;
use crate::config::Config;

pub struct Context<'a> {
Expand All @@ -14,6 +15,7 @@ pub struct Context<'a> {
framework_dir: PathBuf,
include_dir: PathBuf,
system_headers: HashSet<&'static Path>,
pub library_unavailability: Option<Unavailable>,
}

impl<'a> Context<'a> {
Expand All @@ -29,6 +31,7 @@ impl<'a> Context<'a> {
Path::new("objc/NSObject.h"),
Path::new("objc/NSObjCRuntime.h"),
]),
library_unavailability: None,
}
}

Expand Down
33 changes: 16 additions & 17 deletions crates/header-translator/src/library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ use std::fs;
use std::io;
use std::path::Path;

use crate::availability::Unavailable;
use crate::file::{File, FILE_PRELUDE};

#[derive(Debug, PartialEq, Default)]
pub struct Library {
pub files: BTreeMap<String, File>,
pub unavailability: Unavailable,
}

impl Library {
pub fn new() -> Self {
pub(crate) fn new(unavailability: Unavailable) -> Self {
Self {
files: BTreeMap::new(),
unavailability,
}
}

Expand Down Expand Up @@ -60,16 +63,16 @@ impl fmt::Display for Library {
// NOTE: some SDK files have '+' in the file name
let name = name.replace('+', "_");
for stmt in &file.stmts {
let mut iter = stmt.declared_types();
if let Some(item) = iter.next() {
// Use a set to deduplicate features, and to have them in
// a consistent order
let mut features = BTreeSet::new();
stmt.visit_required_types(|item| {
if let Some(feature) = item.feature() {
features.insert(format!("feature = \"{feature}\""));
}
});
// Use a set to deduplicate features, and to have them in
// a consistent order
let mut features = BTreeSet::new();
stmt.visit_required_types(|item| {
if let Some(feature) = item.feature() {
features.insert(format!("feature = \"{feature}\""));
}
});

for (item, unavailability) in stmt.declared_types() {
madsmtm marked this conversation as resolved.
Show resolved Hide resolved
match features.len() {
0 => {}
1 => {
Expand All @@ -87,12 +90,8 @@ impl fmt::Display for Library {
)?;
}
}

writeln!(f, "pub use self::__{name}::{{{item}")?;
for item in iter {
writeln!(f, ", {item}")?;
}
writeln!(f, "}};")?;
write!(f, "{unavailability}")?;
writeln!(f, "pub use self::__{name}::{{{item}}};")?;
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/header-translator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn parse_sdk(index: &Index<'_>, sdk: &SdkPath, llvm_target: &str, config: &Confi
let tu = get_translation_unit(index, sdk, llvm_target);

let mut preprocessing = true;
let mut result = Output::from_libraries(config.libraries.keys());
let mut result = Output::from_libraries(&config.libraries);

let mut library_span = None;
let mut library_span_name = String::new();
Expand Down Expand Up @@ -270,6 +270,7 @@ fn parse_sdk(index: &Index<'_>, sdk: &SdkPath, llvm_target: &str, config: &Confi
preprocessing = false;
// No more includes / macro expansions after this line
let file = library.files.get_mut(&file_name).expect("file");
context.library_unavailability = Some(library.unavailability.clone());
for stmt in Stmt::parse(&entity, &context) {
file.add_stmt(stmt);
}
Expand Down
9 changes: 5 additions & 4 deletions crates/header-translator/src/output.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::collections::HashMap;
use std::collections::{BTreeMap, BTreeSet};
use std::str::FromStr;

use crate::config::Config;
use crate::config::{Config, LibraryData};
use crate::library::Library;
use crate::stmt::Stmt;

Expand All @@ -11,10 +12,10 @@ pub struct Output {
}

impl Output {
pub fn from_libraries(libraries: impl IntoIterator<Item = impl Into<String>>) -> Self {
pub fn from_libraries(libraries: &HashMap<String, LibraryData>) -> Self {
let libraries = libraries
.into_iter()
.map(|name| (name.into(), Library::new()))
.iter()
.map(|(name, library_data)| (name.into(), Library::new(library_data.unavailability())))
.collect();
Self { libraries }
}
Expand Down
Loading