Skip to content

Commit 787d9d0

Browse files
youknowoneyouknowone
authored
Merge pull request RustPython#3643 from youknowone/derive-class-better
Fix not to manually call make_class from modules
2 parents 03b7dd3 + 6ed42f7 commit 787d9d0

File tree

7 files changed

+237
-136
lines changed

7 files changed

+237
-136
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎derive/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@ once_cell = "1.10.0"
2222
textwrap = { version = "0.15.0", default-features = false }
2323
indexmap = "1.8.1"
2424
serde_json = "1.0.79"
25+
itertools = "0.10.3"

‎derive/src/pyclass.rs‎

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ pub(crate) fn impl_pyimpl(attr: AttributeArgs, item: Item) -> Result<TokenStream
107107
} = extract_impl_attrs(attr, &Ident::new(&quote!(ty).to_string(), ty.span()))?;
108108

109109
let getset_impl = &context.getset_items;
110-
let extend_impl = &context.impl_extend_items;
111-
let slots_impl = &context.extend_slots_items;
110+
let extend_impl = context.impl_extend_items.validate()?;
111+
let slots_impl = context.extend_slots_items.validate()?;
112112
let class_extensions = &context.class_extensions;
113113
quote! {
114114
#imp
@@ -143,8 +143,8 @@ pub(crate) fn impl_pyimpl(attr: AttributeArgs, item: Item) -> Result<TokenStream
143143
} = extract_impl_attrs(attr, &trai.ident)?;
144144

145145
let getset_impl = &context.getset_items;
146-
let extend_impl = &context.impl_extend_items;
147-
let slots_impl = &context.extend_slots_items;
146+
let extend_impl = &context.impl_extend_items.validate()?;
147+
let slots_impl = &context.extend_slots_items.validate()?;
148148
let class_extensions = &context.class_extensions;
149149
let extra_methods = iter_chain![
150150
parse_quote! {
@@ -278,6 +278,9 @@ fn generate_class_def(
278278
}
279279

280280
pub(crate) fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStream> {
281+
if matches!(item, syn::Item::Use(_)) {
282+
return Ok(quote!(#item));
283+
}
281284
let (ident, attrs) = pyclass_ident_and_attrs(&item)?;
282285
let fake_ident = Ident::new("pyclass", item.span());
283286
let class_meta = ClassItemMeta::from_nested(ident.clone(), fake_ident, attr.into_iter())?;
@@ -502,9 +505,13 @@ where
502505
}
503506
};
504507

505-
args.context
506-
.impl_extend_items
507-
.add_item(py_name, args.cfgs.to_vec(), tokens)?;
508+
args.context.impl_extend_items.add_item(
509+
ident.clone(),
510+
vec![py_name],
511+
args.cfgs.to_vec(),
512+
tokens,
513+
5,
514+
)?;
508515
Ok(())
509516
}
510517
}
@@ -560,10 +567,13 @@ where
560567
}
561568
};
562569

570+
let pyname = format!("(slot {})", slot_name);
563571
args.context.extend_slots_items.add_item(
564-
format!("(slot {})", slot_name),
572+
ident.clone(),
573+
vec![pyname],
565574
args.cfgs.to_vec(),
566575
tokens,
576+
2,
567577
)?;
568578

569579
Ok(())
@@ -583,32 +593,34 @@ where
583593
let py_name = item_meta.simple_name()?;
584594
Ok(py_name)
585595
};
586-
let (py_name, tokens) = if args.item.function_or_method().is_ok() || args.item.is_const() {
587-
let ident = args.item.get_ident().unwrap();
588-
let py_name = get_py_name(&attr, ident)?;
589-
590-
let value = if args.item.is_const() {
591-
// TODO: ctx.new_value
592-
quote_spanned!(ident.span() => ctx.new_int(Self::#ident).into())
596+
let (ident, py_name, tokens) =
597+
if args.item.function_or_method().is_ok() || args.item.is_const() {
598+
let ident = args.item.get_ident().unwrap();
599+
let py_name = get_py_name(&attr, ident)?;
600+
601+
let value = if args.item.is_const() {
602+
// TODO: ctx.new_value
603+
quote_spanned!(ident.span() => ctx.new_int(Self::#ident).into())
604+
} else {
605+
quote_spanned!(ident.span() => Self::#ident(ctx))
606+
};
607+
(
608+
ident,
609+
py_name.clone(),
610+
quote! {
611+
class.set_str_attr(#py_name, #value);
612+
},
613+
)
593614
} else {
594-
quote_spanned!(ident.span() => Self::#ident(ctx))
615+
return Err(self.new_syn_error(
616+
args.item.span(),
617+
"can only be on a const or an associated method without argument",
618+
));
595619
};
596-
(
597-
py_name.clone(),
598-
quote! {
599-
class.set_str_attr(#py_name, #value);
600-
},
601-
)
602-
} else {
603-
return Err(self.new_syn_error(
604-
args.item.span(),
605-
"can only be on a const or an associated method without argument",
606-
));
607-
};
608620

609621
args.context
610622
.impl_extend_items
611-
.add_item(py_name, cfgs, tokens)?;
623+
.add_item(ident.clone(), vec![py_name], cfgs, tokens, 1)?;
612624

613625
Ok(())
614626
}

‎derive/src/pymodule.rs‎

Lines changed: 100 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub fn impl_pymodule(attr: AttributeArgs, module_item: Item) -> Result<TokenStre
8787

8888
// append additional items
8989
let module_name = context.name.as_str();
90-
let module_extend_items = context.module_extend_items;
90+
let module_extend_items = context.module_extend_items.validate()?;
9191
let doc = doc.or_else(|| {
9292
crate::doc::try_read(module_name)
9393
.ok()
@@ -317,7 +317,7 @@ impl ModuleItem for FunctionItem {
317317
let py_name = item_meta.simple_name()?;
318318
let sig_doc = text_signature(func.sig(), &py_name);
319319

320-
let item = {
320+
let (tokens, py_names) = {
321321
let module = args.module_name();
322322
let doc = args.attrs.doc().or_else(|| {
323323
crate::doc::try_module_item(module, &py_name)
@@ -340,13 +340,16 @@ impl ModuleItem for FunctionItem {
340340
);
341341

342342
if self.pyattrs.is_empty() {
343-
quote_spanned! { ident.span() => {
344-
let func = #new_func;
345-
vm.__module_set_attr(module, #py_name, func).unwrap();
346-
}}
343+
(
344+
quote_spanned! { ident.span() => {
345+
let func = #new_func;
346+
vm.__module_set_attr(module, #py_name, func).unwrap();
347+
}},
348+
vec![py_name],
349+
)
347350
} else {
348351
let mut py_names = HashSet::new();
349-
py_names.insert(py_name.clone());
352+
py_names.insert(py_name);
350353
for attr_index in self.pyattrs.iter().rev() {
351354
let mut loop_unit = || {
352355
let attr_attr = args.attrs.remove(*attr_index);
@@ -368,30 +371,37 @@ impl ModuleItem for FunctionItem {
368371
args.context.errors.ok_or_push(r);
369372
}
370373
let py_names: Vec<_> = py_names.into_iter().collect();
371-
quote_spanned! { ident.span().resolved_at(Span::call_site()) => {
372-
let func = #new_func;
373-
for name in [#(#py_names,)*] {
374-
vm.__module_set_attr(module, name, func.clone()).unwrap();
375-
}
376-
}}
374+
(
375+
quote_spanned! { ident.span().resolved_at(Span::call_site()) => {
376+
let func = #new_func;
377+
for name in [#(#py_names,)*] {
378+
vm.__module_set_attr(module, name, func.clone()).unwrap();
379+
}
380+
}},
381+
py_names,
382+
)
377383
}
378384
};
379385

380-
args.context
381-
.module_extend_items
382-
.add_item(py_name, args.cfgs.to_vec(), item)?;
386+
args.context.module_extend_items.add_item(
387+
ident.clone(),
388+
py_names,
389+
args.cfgs.to_vec(),
390+
tokens,
391+
10,
392+
)?;
383393
Ok(())
384394
}
385395
}
386396

387397
impl ModuleItem for ClassItem {
388398
fn gen_module_item(&self, args: ModuleItemArgs<'_>) -> Result<()> {
389399
let (ident, _) = pyclass_ident_and_attrs(args.item)?;
390-
let (module_name, class_name) = {
400+
let (class_name, class_new) = {
391401
let class_attr = &mut args.attrs[self.inner.index];
402+
let noattr = class_attr.try_remove_name("noattr")?;
392403
if self.pyattrs.is_empty() {
393404
// check noattr before ClassItemMeta::from_attr
394-
let noattr = class_attr.try_remove_name("noattr")?;
395405
if noattr.is_none() {
396406
return Err(syn::Error::new_spanned(
397407
ident,
@@ -403,6 +413,8 @@ impl ModuleItem for ClassItem {
403413
));
404414
}
405415
}
416+
let noattr = noattr.is_some();
417+
let is_use = matches!(&args.item, syn::Item::Use(_));
406418

407419
let class_meta = ClassItemMeta::from_attr(ident.clone(), class_attr)?;
408420
let module_name = args.context.name.clone();
@@ -414,9 +426,19 @@ impl ModuleItem for ClassItem {
414426
})?;
415427
module_name
416428
};
417-
let class_name = class_meta.class_name()?;
418-
(module_name, class_name)
429+
let class_name = if noattr && is_use {
430+
"<NO ATTR>".to_owned()
431+
} else {
432+
class_meta.class_name()?
433+
};
434+
let class_new = quote_spanned!(ident.span() =>
435+
let new_class = <#ident as ::rustpython_vm::pyclass::PyClassImpl>::make_class(&vm.ctx);
436+
new_class.set_str_attr("__module__", vm.new_pyobj(#module_name));
437+
);
438+
(class_name, class_new)
419439
};
440+
441+
let mut py_names = Vec::new();
420442
for attr_index in self.pyattrs.iter().rev() {
421443
let mut loop_unit = || {
422444
let attr_attr = args.attrs.remove(*attr_index);
@@ -425,23 +447,41 @@ impl ModuleItem for ClassItem {
425447
let py_name = item_meta
426448
.optional_name()
427449
.unwrap_or_else(|| class_name.clone());
428-
let new_class = quote_spanned!(ident.span() =>
429-
<#ident as ::rustpython_vm::pyclass::PyClassImpl>::make_class(&vm.ctx);
430-
);
431-
let item = quote! {
432-
let new_class = #new_class;
433-
new_class.set_str_attr("__module__", vm.new_pyobj(#module_name));
434-
vm.__module_set_attr(&module, #py_name, new_class).unwrap();
435-
};
450+
py_names.push(py_name);
436451

437-
args.context
438-
.module_extend_items
439-
.add_item(py_name, args.cfgs.to_vec(), item)?;
440452
Ok(())
441453
};
442454
let r = loop_unit();
443455
args.context.errors.ok_or_push(r);
444456
}
457+
458+
let set_attr = match py_names.len() {
459+
0 => quote! {
460+
let _ = new_class; // suppress warning
461+
},
462+
1 => {
463+
let py_name = &py_names[0];
464+
quote! {
465+
vm.__module_set_attr(&module, #py_name, new_class).unwrap();
466+
}
467+
}
468+
_ => quote! {
469+
for name in [#(#py_names,)*] {
470+
vm.__module_set_attr(&module, name, new_class.clone()).unwrap();
471+
}
472+
},
473+
};
474+
475+
args.context.module_extend_items.add_item(
476+
ident.clone(),
477+
py_names,
478+
args.cfgs.to_vec(),
479+
quote_spanned! { ident.span() =>
480+
#class_new
481+
#set_attr
482+
},
483+
0,
484+
)?;
445485
Ok(())
446486
}
447487
}
@@ -504,7 +544,7 @@ impl ModuleItem for AttributeItem {
504544
return Err(self
505545
.new_syn_error(item.span(), "Only single #[pyattr] is allowed for `use`"));
506546
}
507-
return iter_use_idents(item, |ident, is_unique| {
547+
let _ = iter_use_idents(item, |ident, is_unique| {
508548
let item_meta = SimpleItemMeta::from_attr(ident.clone(), &attr)?;
509549
let py_name = if is_unique {
510550
item_meta.simple_name()?
@@ -520,11 +560,16 @@ impl ModuleItem for AttributeItem {
520560
let tokens = quote_spanned! { ident.span() =>
521561
vm.__module_set_attr(module, #py_name, vm.new_pyobj(#ident)).unwrap();
522562
};
523-
args.context
524-
.module_extend_items
525-
.add_item(py_name, cfgs.clone(), tokens)?;
563+
args.context.module_extend_items.add_item(
564+
ident.clone(),
565+
vec![py_name],
566+
cfgs.clone(),
567+
tokens,
568+
1,
569+
)?;
526570
Ok(())
527-
});
571+
})?;
572+
return Ok(());
528573
}
529574
other => {
530575
return Err(
@@ -533,13 +578,16 @@ impl ModuleItem for AttributeItem {
533578
}
534579
};
535580

536-
let tokens = if self.pyattrs.is_empty() {
537-
quote_spanned! { ident.span() => {
538-
#let_obj
539-
vm.__module_set_attr(module, #py_name, obj).unwrap();
540-
}}
581+
let (tokens, py_names) = if self.pyattrs.is_empty() {
582+
(
583+
quote_spanned! { ident.span() => {
584+
#let_obj
585+
vm.__module_set_attr(module, #py_name, obj).unwrap();
586+
}},
587+
vec![py_name],
588+
)
541589
} else {
542-
let mut names = vec![py_name.clone()];
590+
let mut names = vec![py_name];
543591
for attr_index in self.pyattrs.iter().rev() {
544592
let mut loop_unit = || {
545593
let attr_attr = args.attrs.remove(*attr_index);
@@ -563,17 +611,20 @@ impl ModuleItem for AttributeItem {
563611
let r = loop_unit();
564612
args.context.errors.ok_or_push(r);
565613
}
566-
quote_spanned! { ident.span() => {
567-
#let_obj
568-
for name in [(#(#names,)*)] {
569-
vm.__module_set_attr(module, name, obj.clone()).unwrap();
570-
}
571-
}}
614+
(
615+
quote_spanned! { ident.span() => {
616+
#let_obj
617+
for name in [(#(#names,)*)] {
618+
vm.__module_set_attr(module, name, obj.clone()).unwrap();
619+
}
620+
}},
621+
names,
622+
)
572623
};
573624

574625
args.context
575626
.module_extend_items
576-
.add_item(py_name, cfgs, tokens)?;
627+
.add_item(ident, py_names, cfgs, tokens, 1)?;
577628

578629
Ok(())
579630
}

0 commit comments

Comments
 (0)