Skip to content

Commit 7fb7dde

Browse files
authored
Move TypeHint into a Python expression AST (#5671)
* WIP * Draft: Move TypeHint into a Python expression AST Implement a subset of Python expr AST Adds to the "name" construct a kind to distinguish global (global name that does not need to be resolved against the current module) vs local (names that needs to be resolved). This consolidates the previous builtin/local/module construct by taking into account that modules can be relative (for example if in a #[pyclass] the user set `module=` to the submodule name) Adds also the support for `Callable[[int], float]` and a beginning of constructs to represent constants (currently only None but will be useful for typing.Literal support) TODO: - update the macro code - update the introspection code * Return type: remove one stage of the specialization (#5673) * Draft: Move TypeHint into a Python expression AST Implement a subset of Python expr AST Adds to the "name" construct a kind to distinguish global (global name that does not need to be resolved against the current module) vs local (names that needs to be resolved). This consolidates the previous builtin/local/module construct by taking into account that modules can be relative (for example if in a #[pyclass] the user set `module=` to the submodule name) Adds also the support for `Callable[[int], float]` and a beginning of constructs to represent constants (currently only None but will be useful for typing.Literal support) TODO: - update the macro code - update the introspection code * Introduces macros * Align macros-backend API * Cleaner representation for setters * Implement the introspection side * Fixes tests * Nit * Fixes deleter type annotation
1 parent 0f4c7ca commit 7fb7dde

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+1167
-839
lines changed

newsfragments/5671.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Use general Python expression syntax for introspection type hints

pyo3-introspection/src/introspection.rs

Lines changed: 106 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::model::{
2-
Argument, Arguments, Attribute, Class, Function, Module, PythonIdentifier, TypeHint,
3-
TypeHintExpr, VariableLengthArgument,
2+
Argument, Arguments, Attribute, Class, Constant, Expr, Function, Module, NameKind, Operator,
3+
TypeHint, VariableLengthArgument,
44
};
55
use anyhow::{anyhow, bail, ensure, Context, Result};
66
use goblin::elf::section_header::SHN_XINDEX;
@@ -13,6 +13,7 @@ use goblin::Object;
1313
use serde::de::value::MapAccessDeserializer;
1414
use serde::de::{Error, MapAccess, Visitor};
1515
use serde::{Deserialize, Deserializer};
16+
use std::borrow::Cow;
1617
use std::cmp::Ordering;
1718
use std::collections::HashMap;
1819
use std::path::Path;
@@ -151,7 +152,7 @@ fn convert_members<'a>(
151152
parent: _,
152153
decorators,
153154
returns,
154-
} => functions.push(convert_function(name, arguments, decorators, returns)?),
155+
} => functions.push(convert_function(name, arguments, decorators, returns)),
155156
Chunk::Attribute {
156157
name,
157158
id: _,
@@ -166,25 +167,22 @@ fn convert_members<'a>(
166167
classes.sort_by(|l, r| l.name.cmp(&r.name));
167168
functions.sort_by(|l, r| match l.name.cmp(&r.name) {
168169
Ordering::Equal => {
169-
// We put the getter before the setter. For that, we put @property before the other ones
170-
if l.decorators
171-
.iter()
172-
.any(|d| d.name == "property" && d.module.as_deref() == Some("builtins"))
173-
{
174-
Ordering::Less
175-
} else if r
176-
.decorators
177-
.iter()
178-
.any(|d| d.name == "property" && d.module.as_deref() == Some("builtins"))
179-
{
180-
Ordering::Greater
181-
} else {
182-
// We pick an ordering based on decorators
183-
l.decorators
184-
.iter()
185-
.map(|d| &d.name)
186-
.cmp(r.decorators.iter().map(|d| &d.name))
170+
fn decorator_expr_key(expr: &Expr) -> (u32, Cow<'_, str>) {
171+
// We put plain names before attributes for @property to be before @foo.property
172+
match expr {
173+
Expr::Name { id, .. } => (0, Cow::Borrowed(id)),
174+
Expr::Attribute { value, attr } => {
175+
let (c, v) = decorator_expr_key(value);
176+
(c + 1, Cow::Owned(format!("{v}.{attr}")))
177+
}
178+
_ => (u32::MAX, Cow::Borrowed("")), // We don't care
179+
}
187180
}
181+
// We pick an ordering based on decorators
182+
l.decorators
183+
.iter()
184+
.map(decorator_expr_key)
185+
.cmp(r.decorators.iter().map(decorator_expr_key))
188186
}
189187
o => o,
190188
});
@@ -195,8 +193,8 @@ fn convert_members<'a>(
195193
fn convert_class(
196194
id: &str,
197195
name: &str,
198-
bases: &[ChunkTypeHint],
199-
decorators: &[ChunkTypeHint],
196+
bases: &[ChunkExpr],
197+
decorators: &[ChunkExpr],
200198
chunks_by_id: &HashMap<&str, &Chunk>,
201199
chunks_by_parent: &HashMap<&str, Vec<&Chunk>>,
202200
) -> Result<Class> {
@@ -215,47 +213,22 @@ fn convert_class(
215213
);
216214
Ok(Class {
217215
name: name.into(),
218-
bases: bases
219-
.iter()
220-
.map(convert_python_identifier)
221-
.collect::<Result<_>>()?,
216+
bases: bases.iter().map(convert_expr).collect(),
222217
methods,
223218
attributes,
224-
decorators: decorators
225-
.iter()
226-
.map(convert_python_identifier)
227-
.collect::<Result<_>>()?,
219+
decorators: decorators.iter().map(convert_expr).collect(),
228220
})
229221
}
230222

231-
fn convert_python_identifier(decorator: &ChunkTypeHint) -> Result<PythonIdentifier> {
232-
match convert_type_hint(decorator) {
233-
TypeHint::Plain(id) => Ok(PythonIdentifier {
234-
module: None,
235-
name: id.clone(),
236-
}),
237-
TypeHint::Ast(expr) => {
238-
if let TypeHintExpr::Identifier(i) = expr {
239-
Ok(i)
240-
} else {
241-
bail!("PyO3 introspection currently only support decorators that are identifiers of a Python function, got {expr:?}")
242-
}
243-
}
244-
}
245-
}
246-
247223
fn convert_function(
248224
name: &str,
249225
arguments: &ChunkArguments,
250-
decorators: &[ChunkTypeHint],
226+
decorators: &[ChunkExpr],
251227
returns: &Option<ChunkTypeHint>,
252-
) -> Result<Function> {
253-
Ok(Function {
228+
) -> Function {
229+
Function {
254230
name: name.into(),
255-
decorators: decorators
256-
.iter()
257-
.map(convert_python_identifier)
258-
.collect::<Result<_>>()?,
231+
decorators: decorators.iter().map(convert_expr).collect(),
259232
arguments: Arguments {
260233
positional_only_arguments: arguments.posonlyargs.iter().map(convert_argument).collect(),
261234
arguments: arguments.args.iter().map(convert_argument).collect(),
@@ -270,7 +243,7 @@ fn convert_function(
270243
.map(convert_variable_length_argument),
271244
},
272245
returns: returns.as_ref().map(convert_type_hint),
273-
})
246+
}
274247
}
275248

276249
fn convert_argument(arg: &ChunkArgument) -> Argument {
@@ -302,34 +275,45 @@ fn convert_attribute(
302275

303276
fn convert_type_hint(arg: &ChunkTypeHint) -> TypeHint {
304277
match arg {
305-
ChunkTypeHint::Ast(expr) => TypeHint::Ast(convert_type_hint_expr(expr)),
278+
ChunkTypeHint::Ast(expr) => TypeHint::Ast(convert_expr(expr)),
306279
ChunkTypeHint::Plain(t) => TypeHint::Plain(t.clone()),
307280
}
308281
}
309282

310-
fn convert_type_hint_expr(expr: &ChunkTypeHintExpr) -> TypeHintExpr {
283+
fn convert_expr(expr: &ChunkExpr) -> Expr {
311284
match expr {
312-
ChunkTypeHintExpr::Local { id } => PythonIdentifier {
313-
module: None,
314-
name: id.clone(),
315-
}
316-
.into(),
317-
ChunkTypeHintExpr::Builtin { id } => PythonIdentifier {
318-
module: Some("builtins".into()),
319-
name: id.clone(),
320-
}
321-
.into(),
322-
ChunkTypeHintExpr::Attribute { module, attr } => PythonIdentifier {
323-
module: Some(module.clone()),
324-
name: attr.clone(),
325-
}
326-
.into(),
327-
ChunkTypeHintExpr::Union { elts } => {
328-
TypeHintExpr::Union(elts.iter().map(convert_type_hint_expr).collect())
329-
}
330-
ChunkTypeHintExpr::Subscript { value, slice } => TypeHintExpr::Subscript {
331-
value: Box::new(convert_type_hint_expr(value)),
332-
slice: slice.iter().map(convert_type_hint_expr).collect(),
285+
ChunkExpr::Name { id, kind } => Expr::Name {
286+
id: id.clone(),
287+
kind: match kind {
288+
ChunkNameKind::Local => NameKind::Local,
289+
ChunkNameKind::Global => NameKind::Global,
290+
},
291+
},
292+
ChunkExpr::Attribute { value, attr } => Expr::Attribute {
293+
value: Box::new(convert_expr(value)),
294+
attr: attr.clone(),
295+
},
296+
ChunkExpr::BinOp { left, op, right } => Expr::BinOp {
297+
left: Box::new(convert_expr(left)),
298+
op: match op {
299+
ChunkOperator::BitOr => Operator::BitOr,
300+
},
301+
right: Box::new(convert_expr(right)),
302+
},
303+
ChunkExpr::Subscript { value, slice } => Expr::Subscript {
304+
value: Box::new(convert_expr(value)),
305+
slice: Box::new(convert_expr(slice)),
306+
},
307+
ChunkExpr::Tuple { elts } => Expr::Tuple {
308+
elts: elts.iter().map(convert_expr).collect(),
309+
},
310+
ChunkExpr::List { elts } => Expr::List {
311+
elts: elts.iter().map(convert_expr).collect(),
312+
},
313+
ChunkExpr::Constant { value } => Expr::Constant {
314+
value: match value {
315+
ChunkConstant::None => Constant::None,
316+
},
333317
},
334318
}
335319
}
@@ -451,7 +435,7 @@ fn deserialize_chunk(
451435
})?;
452436
serde_json::from_slice(chunk).with_context(|| {
453437
format!(
454-
"Failed to parse introspection chunk: '{}'",
438+
"Failed to parse introspection chunk: {:?}",
455439
String::from_utf8_lossy(chunk)
456440
)
457441
})
@@ -476,9 +460,9 @@ enum Chunk {
476460
id: String,
477461
name: String,
478462
#[serde(default)]
479-
bases: Vec<ChunkTypeHint>,
463+
bases: Vec<ChunkExpr>,
480464
#[serde(default)]
481-
decorators: Vec<ChunkTypeHint>,
465+
decorators: Vec<ChunkExpr>,
482466
},
483467
Function {
484468
#[serde(default)]
@@ -488,7 +472,7 @@ enum Chunk {
488472
#[serde(default)]
489473
parent: Option<String>,
490474
#[serde(default)]
491-
decorators: Vec<ChunkTypeHint>,
475+
decorators: Vec<ChunkExpr>,
492476
#[serde(default)]
493477
returns: Option<ChunkTypeHint>,
494478
},
@@ -532,7 +516,7 @@ struct ChunkArgument {
532516
///
533517
/// We keep separated type to allow them to evolve independently (this type will need to handle backward compatibility).
534518
enum ChunkTypeHint {
535-
Ast(ChunkTypeHintExpr),
519+
Ast(ChunkExpr),
536520
Plain(String),
537521
}
538522

@@ -577,22 +561,45 @@ impl<'de> Deserialize<'de> for ChunkTypeHint {
577561

578562
#[derive(Deserialize)]
579563
#[serde(tag = "type", rename_all = "lowercase")]
580-
enum ChunkTypeHintExpr {
581-
Local {
582-
id: String,
583-
},
584-
Builtin {
585-
id: String,
586-
},
587-
Attribute {
588-
module: String,
589-
attr: String,
564+
enum ChunkExpr {
565+
/// A constant like `None` or `123`
566+
Constant {
567+
#[serde(flatten)]
568+
value: ChunkConstant,
590569
},
591-
Union {
592-
elts: Vec<ChunkTypeHintExpr>,
593-
},
594-
Subscript {
595-
value: Box<ChunkTypeHintExpr>,
596-
slice: Vec<ChunkTypeHintExpr>,
570+
/// A name
571+
Name { id: String, kind: ChunkNameKind },
572+
/// An attribute `value.attr`
573+
Attribute { value: Box<Self>, attr: String },
574+
/// A binary operator
575+
BinOp {
576+
left: Box<Self>,
577+
op: ChunkOperator,
578+
right: Box<Self>,
597579
},
580+
/// A tuple
581+
Tuple { elts: Vec<Self> },
582+
/// A list
583+
List { elts: Vec<Self> },
584+
/// A subscript `value[slice]`
585+
Subscript { value: Box<Self>, slice: Box<Self> },
586+
}
587+
588+
#[derive(Deserialize)]
589+
#[serde(rename_all = "lowercase")]
590+
enum ChunkNameKind {
591+
Local,
592+
Global,
593+
}
594+
595+
#[derive(Deserialize)]
596+
#[serde(tag = "kind", rename_all = "lowercase")]
597+
pub enum ChunkConstant {
598+
None,
599+
}
600+
601+
#[derive(Deserialize)]
602+
#[serde(rename_all = "lowercase")]
603+
pub enum ChunkOperator {
604+
BitOr,
598605
}

0 commit comments

Comments
 (0)