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

emit c-string literals on Rust 1.77 or later #4269

Merged
merged 3 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions pyo3-build-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ pub fn print_feature_cfgs() {
println!("cargo:rustc-cfg=invalid_from_utf8_lint");
}

if rustc_minor_version >= 77 {
println!("cargo:rustc-cfg=c_str_lit");
}

if rustc_minor_version >= 78 {
println!("cargo:rustc-cfg=diagnostic_namespace");
}
Expand All @@ -165,6 +169,7 @@ pub fn print_expected_cfgs() {
println!("cargo:rustc-check-cfg=cfg(pyo3_disable_reference_pool)");
println!("cargo:rustc-check-cfg=cfg(pyo3_leak_on_drop_without_reference_pool)");
println!("cargo:rustc-check-cfg=cfg(diagnostic_namespace)");
println!("cargo:rustc-check-cfg=cfg(c_str_lit)");

// allow `Py_3_*` cfgs from the minimum supported version up to the
// maximum minor version (+1 for development for the next)
Expand Down
5 changes: 4 additions & 1 deletion pyo3-macros-backend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ pyo3-build-config = { path = "../pyo3-build-config", version = "=0.22.0-dev", fe
quote = { version = "1", default-features = false }

[dependencies.syn]
version = "2"
version = "2.0.59" # for `LitCStr`
default-features = false
features = ["derive", "parsing", "printing", "clone-impls", "full", "extra-traits"]

[build-dependencies]
pyo3-build-config = { path = "../pyo3-build-config", version = "=0.22.0-dev" }

[lints]
workspace = true

Expand Down
4 changes: 4 additions & 0 deletions pyo3-macros-backend/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {
pyo3_build_config::print_expected_cfgs();
pyo3_build_config::print_feature_cfgs();
}
11 changes: 5 additions & 6 deletions pyo3-macros-backend/src/konst.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use std::borrow::Cow;
use std::ffi::CString;

use crate::utils::Ctx;
use crate::utils::{Ctx, LitCStr};
use crate::{
attributes::{self, get_pyo3_options, take_attributes, NameAttribute},
deprecations::Deprecations,
};
use proc_macro2::{Ident, TokenStream};
use quote::quote;
use proc_macro2::{Ident, Span};
use syn::{
ext::IdentExt,
parse::{Parse, ParseStream},
Expand All @@ -29,10 +29,9 @@ impl ConstSpec<'_> {
}

/// Null-terminated Python name
pub fn null_terminated_python_name(&self, ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
pub fn null_terminated_python_name(&self, ctx: &Ctx) -> LitCStr {
let name = self.python_name().to_string();
quote!(#pyo3_path::ffi::c_str!(#name))
LitCStr::new(CString::new(name).unwrap(), Span::call_site(), ctx.clone())
}
}

Expand Down
11 changes: 5 additions & 6 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use std::borrow::Cow;
use std::ffi::CString;
use std::fmt::Display;

use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote, quote_spanned, ToTokens};
use syn::{ext::IdentExt, spanned::Spanned, Ident, Result};

use crate::deprecations::deprecate_trailing_option_default;
use crate::utils::Ctx;
use crate::utils::{Ctx, LitCStr};
use crate::{
attributes::{FromPyWithAttribute, TextSignatureAttribute, TextSignatureAttributeValue},
deprecations::{Deprecation, Deprecations},
Expand Down Expand Up @@ -472,12 +473,10 @@ impl<'a> FnSpec<'a> {
})
}

pub fn null_terminated_python_name(&self, ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let span = self.python_name.span();
let pyo3_path = pyo3_path.to_tokens_spanned(span);
pub fn null_terminated_python_name(&self, ctx: &Ctx) -> LitCStr {
let name = self.python_name.to_string();
quote_spanned!(self.python_name.span() => #pyo3_path::ffi::c_str!(#name))
let name = CString::new(name).unwrap();
LitCStr::new(name, self.python_name.span(), ctx.clone())
}

fn parse_fn_type(
Expand Down
8 changes: 5 additions & 3 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use crate::{
get_doc,
pyclass::PyClassPyO3Option,
pyfunction::{impl_wrap_pyfunction, PyFunctionOptions},
utils::Ctx,
utils::{Ctx, LitCStr},
};
use proc_macro2::TokenStream;
use proc_macro2::{Span, TokenStream};
use quote::quote;
use std::ffi::CString;
use syn::{
ext::IdentExt,
parse::{Parse, ParseStream},
Expand Down Expand Up @@ -403,10 +404,11 @@ fn module_initialization(name: &syn::Ident, ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let pyinit_symbol = format!("PyInit_{}", name);
let name = name.to_string();
let pyo3_name = LitCStr::new(CString::new(name).unwrap(), Span::call_site(), ctx.clone());

quote! {
#[doc(hidden)]
pub const __PYO3_NAME: &'static ::std::ffi::CStr = #pyo3_path::ffi::c_str!(#name);
pub const __PYO3_NAME: &'static ::std::ffi::CStr = #pyo3_name;

pub(super) struct MakeDef;
#[doc(hidden)]
Expand Down
4 changes: 2 additions & 2 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::pymethod::{
SlotDef, __GETITEM__, __HASH__, __INT__, __LEN__, __REPR__, __RICHCMP__,
};
use crate::pyversions;
use crate::utils::{self, apply_renaming_rule, PythonDoc};
use crate::utils::{self, apply_renaming_rule, LitCStr, PythonDoc};
use crate::utils::{is_abi3, Ctx};
use crate::PyFunctionOptions;

Expand Down Expand Up @@ -2016,7 +2016,7 @@ impl<'a> PyClassImplsBuilder<'a> {
let Ctx { pyo3_path, .. } = ctx;
let cls = self.cls;
let doc = self.doc.as_ref().map_or(
quote! {#pyo3_path::ffi::c_str!("")},
LitCStr::empty(ctx.clone()).to_token_stream(),
PythonDoc::to_token_stream,
);
let is_basetype = self.attr.options.subclass.is_some();
Expand Down
9 changes: 5 additions & 4 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::borrow::Cow;
use std::ffi::CString;

use crate::attributes::{NameAttribute, RenamingRule};
use crate::deprecations::deprecate_trailing_option_default;
use crate::method::{CallingConvention, ExtractErrorMode, PyArg};
use crate::params::{check_arg_for_gil_refs, impl_regular_arg_param, Holders};
use crate::utils::Ctx;
use crate::utils::PythonDoc;
use crate::utils::{Ctx, LitCStr};
use crate::{
method::{FnArg, FnSpec, FnType, SelfType},
pyfunction::PyFunctionOptions,
Expand Down Expand Up @@ -870,8 +871,7 @@ pub enum PropertyType<'a> {
}

impl PropertyType<'_> {
fn null_terminated_python_name(&self, ctx: &Ctx) -> Result<TokenStream> {
let Ctx { pyo3_path, .. } = ctx;
fn null_terminated_python_name(&self, ctx: &Ctx) -> Result<LitCStr> {
match self {
PropertyType::Descriptor {
field,
Expand All @@ -892,7 +892,8 @@ impl PropertyType<'_> {
bail_spanned!(field.span() => "`get` and `set` with tuple struct fields require `name`");
}
};
Ok(quote_spanned!(field.span() => #pyo3_path::ffi::c_str!(#name)))
let name = CString::new(name).unwrap();
Ok(LitCStr::new(name, field.span(), ctx.clone()))
}
PropertyType::Function { spec, .. } => Ok(spec.null_terminated_python_name(ctx)),
}
Expand Down
74 changes: 65 additions & 9 deletions pyo3-macros-backend/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::attributes::{CrateAttribute, RenamingRule};
use proc_macro2::{Span, TokenStream};
use quote::{quote, ToTokens};
use std::ffi::CString;
use syn::spanned::Spanned;
use syn::{punctuated::Punctuated, Token};

Expand Down Expand Up @@ -67,14 +68,58 @@ pub fn option_type_argument(ty: &syn::Type) -> Option<&syn::Type> {
None
}

// TODO: Replace usage of this by [`syn::LitCStr`] when on MSRV 1.77
#[derive(Clone)]
pub struct LitCStr {
lit: CString,
span: Span,
#[allow(dead_code)]
Icxolu marked this conversation as resolved.
Show resolved Hide resolved
ctx: Ctx,
Icxolu marked this conversation as resolved.
Show resolved Hide resolved
}

impl LitCStr {
pub fn new(lit: CString, span: Span, ctx: Ctx) -> Self {
Self { lit, span, ctx }
}

pub fn empty(ctx: Ctx) -> Self {
Self {
lit: CString::new("").unwrap(),
span: Span::call_site(),
ctx,
}
}
}

impl quote::ToTokens for LitCStr {
#[cfg(c_str_lit)]
fn to_tokens(&self, tokens: &mut TokenStream) {
syn::LitCStr::new(&self.lit, self.span).to_tokens(tokens);
}

#[cfg(not(c_str_lit))]
fn to_tokens(&self, tokens: &mut TokenStream) {
let Ctx { pyo3_path, .. } = &self.ctx;
let lit = self.lit.to_str().unwrap();
tokens.extend(quote::quote_spanned!(self.span => #pyo3_path::ffi::c_str!(#lit)));
}
}
Icxolu marked this conversation as resolved.
Show resolved Hide resolved

/// A syntax tree which evaluates to a nul-terminated docstring for Python.
///
/// Typically the tokens will just be that string, but if the original docs included macro
/// expressions then the tokens will be a concat!("...", "\n", "\0") expression of the strings and
/// macro parts.
/// contents such as parse the string contents.
/// macro parts. contents such as parse the string contents.
#[derive(Clone)]
pub struct PythonDoc(PythonDocKind);

#[derive(Clone)]
pub struct PythonDoc(TokenStream);
enum PythonDocKind {
LitCStr(LitCStr),
// There is currently no way to `concat!` c-string literals, we fallback to the `c_str!` macro in
// this case.
Tokens(TokenStream),
Copy link
Member

Choose a reason for hiding this comment

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

Agreed I think this case will have to stick around for the foreseeable future, but I think we would at least eventually be able to just use something like CStr::from_bytes_with_nul (which is const since 1.72).

Copy link
Member

Choose a reason for hiding this comment

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

... or maybe #4270 is good enough for now :)

}

/// Collects all #[doc = "..."] attributes into a TokenStream evaluating to a null-terminated string.
///
Expand Down Expand Up @@ -125,7 +170,7 @@ pub fn get_doc(
}
}

let tokens = if !parts.is_empty() {
if !parts.is_empty() {
// Doc contained macro pieces - return as `concat!` expression
if !current_part.is_empty() {
parts.push(current_part.to_token_stream());
Expand All @@ -140,17 +185,26 @@ pub fn get_doc(
syn::token::Comma(Span::call_site()).to_tokens(tokens);
});

tokens
PythonDoc(PythonDocKind::Tokens(
quote!(#pyo3_path::ffi::c_str!(#tokens)),
))
} else {
// Just a string doc - return directly with nul terminator
current_part.to_token_stream()
};
PythonDoc(quote!(#pyo3_path::ffi::c_str!(#tokens)))
let docs = CString::new(current_part).unwrap();
PythonDoc(PythonDocKind::LitCStr(LitCStr::new(
docs,
Span::call_site(),
ctx.clone(),
)))
}
}

impl quote::ToTokens for PythonDoc {
fn to_tokens(&self, tokens: &mut TokenStream) {
self.0.to_tokens(tokens)
match &self.0 {
PythonDocKind::LitCStr(lit) => lit.to_tokens(tokens),
PythonDocKind::Tokens(toks) => toks.to_tokens(tokens),
}
}
}

Expand All @@ -161,6 +215,7 @@ pub fn unwrap_ty_group(mut ty: &syn::Type) -> &syn::Type {
ty
}

#[derive(Clone)]
pub struct Ctx {
/// Where we can find the pyo3 crate
pub pyo3_path: PyO3CratePath,
Expand Down Expand Up @@ -194,6 +249,7 @@ impl Ctx {
}
}

#[derive(Clone)]
pub enum PyO3CratePath {
Given(syn::Path),
Default,
Expand Down
Loading