Skip to content

Commit ed81105

Browse files
coolreader18coolreader18
committed
Apply review suggestions
1 parent 7ba470e commit ed81105

File tree

7 files changed

+35
-55
lines changed

7 files changed

+35
-55
lines changed

‎compiler/src/compile.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,10 @@ impl CodeInfo {
6060
.cellvars
6161
.iter()
6262
.map(|var| {
63-
for (i, arg) in all_args.iter().enumerate() {
64-
if var == arg {
65-
found_cellarg = true;
66-
return i as isize;
67-
}
68-
}
69-
-1
63+
all_args.iter().position(|arg| var == arg).map_or(-1, |i| {
64+
found_cellarg = true;
65+
i as isize
66+
})
7067
})
7168
.collect::<Box<[_]>>();
7269
if found_cellarg {

‎compiler/src/symboltable.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ pub struct Symbol {
108108
pub is_parameter: bool,
109109
pub is_annotated: bool,
110110
pub is_imported: bool,
111+
pub is_nonlocal: bool,
111112

112113
// indicates if the symbol gets a value assigned by a named expression in a comprehension
113114
// this is required to correct the scope in the analysis.
@@ -139,6 +140,7 @@ impl Symbol {
139140
is_parameter: false,
140141
is_annotated: false,
141142
is_imported: false,
143+
is_nonlocal: false,
142144
is_assign_namedexpr_in_comprehension: false,
143145
is_iter: false,
144146
is_free_class: false,
@@ -1162,6 +1164,7 @@ impl SymbolTableBuilder {
11621164
match role {
11631165
SymbolUsage::Nonlocal => {
11641166
symbol.scope = SymbolScope::Free;
1167+
symbol.is_nonlocal = true;
11651168
}
11661169
SymbolUsage::Imported => {
11671170
symbol.is_assigned = true;

‎derive/src/pyclass.rs

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -615,9 +615,7 @@ impl PropertyItemMeta {
615615
fn property_name(&self) -> Result<(String, GetSetItemKind)> {
616616
let inner = self.inner();
617617
let magic = inner._bool("magic")?;
618-
let setter = inner._bool("setter")?;
619-
let deleter = inner._bool("deleter")?;
620-
let kind = match (setter, deleter) {
618+
let kind = match (inner._bool("setter")?, inner._bool("deleter")?) {
621619
(false, false) => GetSetItemKind::Get,
622620
(true, false) => GetSetItemKind::Set,
623621
(false, true) => GetSetItemKind::Delete,
@@ -636,19 +634,21 @@ impl PropertyItemMeta {
636634
name
637635
} else {
638636
let sig_name = inner.item_name();
639-
let name = if setter {
640-
if let Some(name) = sig_name.strip_prefix("set_") {
637+
let extract_prefix_name = |prefix, item_typ| {
638+
if let Some(name) = sig_name.strip_prefix(prefix) {
641639
if name.is_empty() {
642640
return Err(syn::Error::new_spanned(
643641
&inner.meta_ident,
644642
format!(
645-
"A #[{}(setter)] fn with a set_* name must \
646-
have something after \"set_\"",
647-
inner.meta_name()
643+
"A #[{}({typ})] fn with a {prefix}* name must \
644+
have something after \"{prefix}\"",
645+
inner.meta_name(),
646+
typ = item_typ,
647+
prefix = prefix
648648
),
649649
));
650650
}
651-
name.to_string()
651+
Ok(name.to_owned())
652652
} else {
653653
return Err(syn::Error::new_spanned(
654654
&inner.meta_ident,
@@ -659,31 +659,11 @@ impl PropertyItemMeta {
659659
),
660660
));
661661
}
662-
} else if deleter {
663-
if let Some(name) = sig_name.strip_prefix("del_") {
664-
if name.is_empty() {
665-
return Err(syn::Error::new_spanned(
666-
&inner.meta_ident,
667-
format!(
668-
"A #[{}(deleter)] fn with a del_* name must \
669-
have something after \"del_\"",
670-
inner.meta_name()
671-
),
672-
));
673-
}
674-
name.to_string()
675-
} else {
676-
return Err(syn::Error::new_spanned(
677-
&inner.meta_ident,
678-
format!(
679-
"A #[{}(deleter)] fn must either have a `name` \
680-
parameter or a fn name along the lines of \"del_*\"",
681-
inner.meta_name()
682-
),
683-
));
684-
}
685-
} else {
686-
sig_name
662+
};
663+
let name = match kind {
664+
GetSetItemKind::Get => sig_name,
665+
GetSetItemKind::Set => extract_prefix_name("set_", "setter")?,
666+
GetSetItemKind::Delete => extract_prefix_name("del_", "deleter")?,
687667
};
688668
if magic {
689669
format!("__{}__", name)

‎vm/src/builtins/dict.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,7 @@ impl PyDictRef {
492492
}
493493
}
494494

495+
#[cold]
495496
fn _subcls_getitem_option(
496497
&self,
497498
key: PyObjectRef,
@@ -504,7 +505,7 @@ impl PyDictRef {
504505
}
505506
}
506507

507-
pub fn get2<K: IntoPyObject + DictKey + Clone>(
508+
pub fn get_chain<K: IntoPyObject + DictKey + Clone>(
508509
&self,
509510
other: &Self,
510511
key: K,
@@ -513,7 +514,7 @@ impl PyDictRef {
513514
let self_exact = self.class().is(&vm.ctx.types.dict_type);
514515
let other_exact = self.class().is(&vm.ctx.types.dict_type);
515516
if self_exact && other_exact {
516-
self.entries.get2(&other.entries, vm, &key)
517+
self.entries.get_chain(&other.entries, vm, &key)
517518
} else if let Some(value) = self._get_item_option_inner(key.clone(), self_exact, vm)? {
518519
Ok(Some(value))
519520
} else {

‎vm/src/dictdatatype.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ impl<T: Clone> Dict<T> {
284284
Ok(ret)
285285
}
286286

287-
pub fn get2<K: DictKey>(
287+
pub fn get_chain<K: DictKey>(
288288
&self,
289289
other: &Self,
290290
vm: &VirtualMachine,

‎vm/src/frame.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -470,22 +470,14 @@ impl ExecutingFrame<'_> {
470470
let x = self.locals.get_item_option(name.clone(), vm)?;
471471
let x = match x {
472472
Some(x) => x,
473-
None => self
474-
.globals
475-
.get2(self.builtins, name.clone(), vm)?
476-
.ok_or_else(|| {
477-
vm.new_name_error(format!("name '{}' is not defined", name))
478-
})?,
473+
None => self.load_global_or_builtin(name, vm)?,
479474
};
480475
self.push_value(x);
481476
Ok(None)
482477
}
483478
bytecode::Instruction::LoadGlobal(idx) => {
484479
let name = &self.code.names[*idx];
485-
let x = self
486-
.globals
487-
.get2(self.builtins, name.clone(), vm)?
488-
.ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?;
480+
let x = self.load_global_or_builtin(name, vm)?;
489481
self.push_value(x);
490482
Ok(None)
491483
}
@@ -952,6 +944,13 @@ impl ExecutingFrame<'_> {
952944
}
953945
}
954946

947+
#[inline]
948+
fn load_global_or_builtin(&self, name: &PyStrRef, vm: &VirtualMachine) -> PyResult {
949+
self.globals
950+
.get_chain(self.builtins, name.clone(), vm)?
951+
.ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))
952+
}
953+
955954
#[cfg_attr(feature = "flame-it", flame("Frame"))]
956955
fn get_elements(
957956
&mut self,

‎vm/src/stdlib/symtable.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ mod decl {
205205

206206
#[pymethod(name = "is_nonlocal")]
207207
fn is_nonlocal(&self) -> bool {
208-
matches!(self.symbol.scope, SymbolScope::Free)
208+
self.symbol.is_nonlocal
209209
}
210210

211211
#[pymethod(name = "is_referenced")]

0 commit comments

Comments
 (0)