Skip to content

Commit 38a735a

Browse files
coolreader18coolreader18
committed
Tidy up -derive, make use of let-else
1 parent c01f014 commit 38a735a

File tree

9 files changed

+232
-284
lines changed

9 files changed

+232
-284
lines changed

‎Cargo.lock

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

‎derive/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ edition = "2021"
1111
proc-macro = true
1212

1313
[dependencies]
14-
rustpython-codegen = { path = "../compiler/codegen", version = "0.1.1" }
1514
rustpython-compiler = { path = "../compiler", version = "0.1.1" }
1615
rustpython-compiler-core = { path = "../compiler/core", version = "0.1.1" }
1716
rustpython-doc = { git = "https://github.com/RustPython/__doc__", branch = "main" }

‎derive/src/compile_bytecode.rs

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ use crate::{extract_spans, Diagnostic};
1717
use once_cell::sync::Lazy;
1818
use proc_macro2::{Span, TokenStream};
1919
use quote::quote;
20-
use rustpython_codegen as codegen;
21-
use rustpython_compiler::compile;
2220
use rustpython_compiler_core::{CodeObject, FrozenModule, Mode};
2321
use std::{
2422
collections::HashMap,
@@ -51,15 +49,25 @@ struct CompilationSource {
5149
span: (Span, Span),
5250
}
5351

52+
pub trait Compiler {
53+
fn compile(
54+
&self,
55+
source: &str,
56+
mode: Mode,
57+
module_name: String,
58+
) -> Result<CodeObject, Box<dyn std::error::Error>>;
59+
}
60+
5461
impl CompilationSource {
5562
fn compile_string<D: std::fmt::Display, F: FnOnce() -> D>(
5663
&self,
5764
source: &str,
5865
mode: Mode,
5966
module_name: String,
67+
compiler: &dyn Compiler,
6068
origin: F,
6169
) -> Result<CodeObject, Diagnostic> {
62-
compile(source, mode, module_name, codegen::CompileOpts::default()).map_err(|err| {
70+
compiler.compile(source, mode, module_name).map_err(|err| {
6371
Diagnostic::spans_error(
6472
self.span,
6573
format!("Python compile error from {}: {}", origin(), err),
@@ -71,21 +79,30 @@ impl CompilationSource {
7179
&self,
7280
mode: Mode,
7381
module_name: String,
82+
compiler: &dyn Compiler,
7483
) -> Result<HashMap<String, FrozenModule>, Diagnostic> {
7584
match &self.kind {
76-
CompilationSourceKind::Dir(rel_path) => {
77-
self.compile_dir(&CARGO_MANIFEST_DIR.join(rel_path), String::new(), mode)
78-
}
85+
CompilationSourceKind::Dir(rel_path) => self.compile_dir(
86+
&CARGO_MANIFEST_DIR.join(rel_path),
87+
String::new(),
88+
mode,
89+
compiler,
90+
),
7991
_ => Ok(hashmap! {
8092
module_name.clone() => FrozenModule {
81-
code: self.compile_single(mode, module_name)?,
93+
code: self.compile_single(mode, module_name, compiler)?,
8294
package: false,
8395
},
8496
}),
8597
}
8698
}
8799

88-
fn compile_single(&self, mode: Mode, module_name: String) -> Result<CodeObject, Diagnostic> {
100+
fn compile_single(
101+
&self,
102+
mode: Mode,
103+
module_name: String,
104+
compiler: &dyn Compiler,
105+
) -> Result<CodeObject, Diagnostic> {
89106
match &self.kind {
90107
CompilationSourceKind::File(rel_path) => {
91108
let path = CARGO_MANIFEST_DIR.join(rel_path);
@@ -95,10 +112,10 @@ impl CompilationSource {
95112
format!("Error reading file {:?}: {}", path, err),
96113
)
97114
})?;
98-
self.compile_string(&source, mode, module_name, || rel_path.display())
115+
self.compile_string(&source, mode, module_name, compiler, || rel_path.display())
99116
}
100117
CompilationSourceKind::SourceCode(code) => {
101-
self.compile_string(&textwrap::dedent(code), mode, module_name, || {
118+
self.compile_string(&textwrap::dedent(code), mode, module_name, compiler, || {
102119
"string literal"
103120
})
104121
}
@@ -113,6 +130,7 @@ impl CompilationSource {
113130
path: &Path,
114131
parent: String,
115132
mode: Mode,
133+
compiler: &dyn Compiler,
116134
) -> Result<HashMap<String, FrozenModule>, Diagnostic> {
117135
let mut code_map = HashMap::new();
118136
let paths = fs::read_dir(path)
@@ -144,6 +162,7 @@ impl CompilationSource {
144162
format!("{}.{}", parent, file_name)
145163
},
146164
mode,
165+
compiler,
147166
)?);
148167
} else if file_name.ends_with(".py") {
149168
let stem = path.file_stem().unwrap().to_str().unwrap();
@@ -163,7 +182,7 @@ impl CompilationSource {
163182
format!("Error reading file {:?}: {}", path, err),
164183
)
165184
})?;
166-
self.compile_string(&source, mode, module_name.clone(), || {
185+
self.compile_string(&source, mode, module_name.clone(), compiler, || {
167186
path.strip_prefix(&*CARGO_MANIFEST_DIR)
168187
.ok()
169188
.unwrap_or(&path)
@@ -239,35 +258,28 @@ impl PyCompileInput {
239258
Some(ident) => ident,
240259
None => continue,
241260
};
261+
let check_str = || match &name_value.lit {
262+
Lit::Str(s) => Ok(s),
263+
_ => Err(err_span!(name_value.lit, "{ident} must be a string")),
264+
};
242265
if ident == "mode" {
243-
match &name_value.lit {
244-
Lit::Str(s) => match s.value().parse() {
245-
Ok(mode_val) => mode = Some(mode_val),
246-
Err(e) => bail_span!(s, "{}", e),
247-
},
248-
_ => bail_span!(name_value.lit, "mode must be a string"),
266+
let s = check_str()?;
267+
match s.value().parse() {
268+
Ok(mode_val) => mode = Some(mode_val),
269+
Err(e) => bail_span!(s, "{}", e),
249270
}
250271
} else if ident == "module_name" {
251-
module_name = Some(match &name_value.lit {
252-
Lit::Str(s) => s.value(),
253-
_ => bail_span!(name_value.lit, "module_name must be string"),
254-
})
272+
module_name = Some(check_str()?.value())
255273
} else if ident == "source" {
256274
assert_source_empty(&source)?;
257-
let code = match &name_value.lit {
258-
Lit::Str(s) => s.value(),
259-
_ => bail_span!(name_value.lit, "source must be a string"),
260-
};
275+
let code = check_str()?.value();
261276
source = Some(CompilationSource {
262277
kind: CompilationSourceKind::SourceCode(code),
263278
span: extract_spans(&name_value).unwrap(),
264279
});
265280
} else if ident == "file" {
266281
assert_source_empty(&source)?;
267-
let path = match &name_value.lit {
268-
Lit::Str(s) => PathBuf::from(s.value()),
269-
_ => bail_span!(name_value.lit, "source must be a string"),
270-
};
282+
let path = check_str()?.value().into();
271283
source = Some(CompilationSource {
272284
kind: CompilationSourceKind::File(path),
273285
span: extract_spans(&name_value).unwrap(),
@@ -278,19 +290,13 @@ impl PyCompileInput {
278290
}
279291

280292
assert_source_empty(&source)?;
281-
let path = match &name_value.lit {
282-
Lit::Str(s) => PathBuf::from(s.value()),
283-
_ => bail_span!(name_value.lit, "source must be a string"),
284-
};
293+
let path = check_str()?.value().into();
285294
source = Some(CompilationSource {
286295
kind: CompilationSourceKind::Dir(path),
287296
span: extract_spans(&name_value).unwrap(),
288297
});
289298
} else if ident == "crate_name" {
290-
let name = match &name_value.lit {
291-
Lit::Str(s) => s.parse()?,
292-
_ => bail_span!(name_value.lit, "source must be a string"),
293-
};
299+
let name = check_str()?.parse()?;
294300
crate_name = Some(name);
295301
}
296302
}
@@ -351,12 +357,17 @@ struct PyCompileArgs {
351357
crate_name: syn::Path,
352358
}
353359

354-
pub fn impl_py_compile(input: TokenStream) -> Result<TokenStream, Diagnostic> {
360+
pub fn impl_py_compile(
361+
input: TokenStream,
362+
compiler: &dyn Compiler,
363+
) -> Result<TokenStream, Diagnostic> {
355364
let input: PyCompileInput = parse2(input)?;
356365
let args = input.parse(false)?;
357366

358367
let crate_name = args.crate_name;
359-
let code = args.source.compile_single(args.mode, args.module_name)?;
368+
let code = args
369+
.source
370+
.compile_single(args.mode, args.module_name, compiler)?;
360371

361372
let bytes = code.to_bytes();
362373
let bytes = LitByteStr::new(&bytes, Span::call_site());
@@ -369,12 +380,15 @@ pub fn impl_py_compile(input: TokenStream) -> Result<TokenStream, Diagnostic> {
369380
Ok(output)
370381
}
371382

372-
pub fn impl_py_freeze(input: TokenStream) -> Result<TokenStream, Diagnostic> {
383+
pub fn impl_py_freeze(
384+
input: TokenStream,
385+
compiler: &dyn Compiler,
386+
) -> Result<TokenStream, Diagnostic> {
373387
let input: PyCompileInput = parse2(input)?;
374388
let args = input.parse(true)?;
375389

376390
let crate_name = args.crate_name;
377-
let code_map = args.source.compile(args.mode, args.module_name)?;
391+
let code_map = args.source.compile(args.mode, args.module_name, compiler)?;
378392

379393
let data =
380394
rustpython_compiler_core::frozen_lib::encode_lib(code_map.iter().map(|(k, v)| (&**k, v)));

‎derive/src/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use syn::parse::Error;
3333

3434
macro_rules! err_span {
3535
($span:expr, $($msg:tt)*) => (
36-
syn::Error::new_spanned(&$span, format!($($msg)*))
36+
syn::Error::new_spanned(&$span, format_args!($($msg)*))
3737
)
3838
}
3939

‎derive/src/from_args.rs

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::util::path_eq;
21
use proc_macro2::TokenStream;
32
use quote::{quote, ToTokens};
43
use syn::{
@@ -39,39 +38,39 @@ impl ArgAttribute {
3938
if !attr.path.is_ident("pyarg") {
4039
return None;
4140
}
42-
let inner = move || match attr.parse_meta()? {
43-
Meta::List(list) => {
44-
let mut iter = list.nested.iter();
45-
let first_arg = iter.next().ok_or_else(|| {
46-
err_span!(list, "There must be at least one argument to #[pyarg()]")
47-
})?;
48-
let kind = match first_arg {
49-
NestedMeta::Meta(Meta::Path(path)) => {
50-
path.get_ident().and_then(ParameterKind::from_ident)
51-
}
52-
_ => None,
53-
};
54-
let kind = kind.ok_or_else(|| {
55-
err_span!(
56-
first_arg,
57-
"The first argument to #[pyarg()] must be the parameter type, either \
41+
let inner = move || {
42+
let Meta::List(list) = attr.parse_meta()? else {
43+
bail_span!(attr, "pyarg must be a list, like #[pyarg(...)]")
44+
};
45+
let mut iter = list.nested.iter();
46+
let first_arg = iter.next().ok_or_else(|| {
47+
err_span!(list, "There must be at least one argument to #[pyarg()]")
48+
})?;
49+
let kind = match first_arg {
50+
NestedMeta::Meta(Meta::Path(path)) => {
51+
path.get_ident().and_then(ParameterKind::from_ident)
52+
}
53+
_ => None,
54+
};
55+
let kind = kind.ok_or_else(|| {
56+
err_span!(
57+
first_arg,
58+
"The first argument to #[pyarg()] must be the parameter type, either \
5859
'positional', 'any', 'named', or 'flatten'."
59-
)
60-
})?;
60+
)
61+
})?;
6162

62-
let mut attribute = ArgAttribute {
63-
name: None,
64-
kind,
65-
default: None,
66-
};
63+
let mut attribute = ArgAttribute {
64+
name: None,
65+
kind,
66+
default: None,
67+
};
6768

68-
for arg in iter {
69-
attribute.parse_argument(arg)?;
70-
}
71-
72-
Ok(attribute)
69+
for arg in iter {
70+
attribute.parse_argument(arg)?;
7371
}
74-
_ => bail_span!(attr, "pyarg must be a list, like #[pyarg(...)]"),
72+
73+
Ok(attribute)
7574
};
7675
Some(inner())
7776
}
@@ -82,7 +81,7 @@ impl ArgAttribute {
8281
}
8382
match arg {
8483
NestedMeta::Meta(Meta::Path(path)) => {
85-
if path_eq(path, "default") || path_eq(path, "optional") {
84+
if path.is_ident("default") || path.is_ident("optional") {
8685
if self.default.is_none() {
8786
self.default = Some(None);
8887
}
@@ -91,7 +90,7 @@ impl ArgAttribute {
9190
}
9291
}
9392
NestedMeta::Meta(Meta::NameValue(name_value)) => {
94-
if path_eq(&name_value.path, "default") {
93+
if name_value.path.is_ident("default") {
9594
if matches!(self.default, Some(Some(_))) {
9695
bail_span!(name_value, "Default already set");
9796
}
@@ -100,7 +99,7 @@ impl ArgAttribute {
10099
Lit::Str(ref val) => self.default = Some(Some(val.parse()?)),
101100
_ => bail_span!(name_value, "Expected string value for default argument"),
102101
}
103-
} else if path_eq(&name_value.path, "name") {
102+
} else if name_value.path.is_ident("name") {
104103
if self.name.is_some() {
105104
bail_span!(name_value, "already have a name")
106105
}

‎derive/src/lib.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,30 @@ pub fn pystruct_sequence_try_from_object(
104104
result_to_tokens(pystructseq::impl_pystruct_sequence_try_from_object(input))
105105
}
106106

107+
// would be cool to move all the macro implementation to a separate rustpython-derive-shared
108+
// that just depends on rustpython-compiler-core, and then rustpython-derive would hook -compiler
109+
// up to it; so that (the bulk of) rustpython-derive and rustpython-codegen could build in parallel
110+
struct Compiler;
111+
impl compile_bytecode::Compiler for Compiler {
112+
fn compile(
113+
&self,
114+
source: &str,
115+
mode: rustpython_compiler_core::Mode,
116+
module_name: String,
117+
) -> Result<rustpython_compiler_core::CodeObject, Box<dyn std::error::Error>> {
118+
use rustpython_compiler::{compile, CompileOpts};
119+
Ok(compile(source, mode, module_name, CompileOpts::default())?)
120+
}
121+
}
122+
107123
#[proc_macro]
108124
pub fn py_compile(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
109-
result_to_tokens(compile_bytecode::impl_py_compile(input.into()))
125+
result_to_tokens(compile_bytecode::impl_py_compile(input.into(), &Compiler))
110126
}
111127

112128
#[proc_macro]
113129
pub fn py_freeze(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
114-
result_to_tokens(compile_bytecode::impl_py_freeze(input.into()))
130+
result_to_tokens(compile_bytecode::impl_py_freeze(input.into(), &Compiler))
115131
}
116132

117133
#[proc_macro_derive(PyPayload)]

0 commit comments

Comments
 (0)