From 5b073154c3f815adde00255f77c431e3b1825a0c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 4 Oct 2021 13:20:40 +0900 Subject: [PATCH 1/2] Defer conversion to Diagnostic as much as possible --- derive/src/compile_bytecode.rs | 8 +++---- derive/src/error.rs | 22 ++---------------- derive/src/from_args.rs | 19 +++++++-------- derive/src/lib.rs | 42 ++++++++++++++++++++++------------ derive/src/pyclass.rs | 31 +++++++------------------ derive/src/pymodule.rs | 5 +--- derive/src/pystructseq.rs | 7 ++---- derive/src/pyvalue.rs | 5 ++-- 8 files changed, 57 insertions(+), 82 deletions(-) diff --git a/derive/src/compile_bytecode.rs b/derive/src/compile_bytecode.rs index e6f3b7c7a..9b361a516 100644 --- a/derive/src/compile_bytecode.rs +++ b/derive/src/compile_bytecode.rs @@ -15,7 +15,7 @@ use crate::{extract_spans, Diagnostic}; use once_cell::sync::Lazy; -use proc_macro2::{Span, TokenStream as TokenStream2}; +use proc_macro2::{Span, TokenStream}; use quote::quote; use rustpython_bytecode::{CodeObject, FrozenModule}; use rustpython_compiler as compile; @@ -281,7 +281,7 @@ impl PyCompileInput { } let source = source.ok_or_else(|| { - Diagnostic::span_error( + syn::Error::new( self.span, "Must have either file or source in py_compile!()/py_freeze!()", ) @@ -335,7 +335,7 @@ struct PyCompileArgs { crate_name: syn::Path, } -pub fn impl_py_compile(input: TokenStream2) -> Result { +pub fn impl_py_compile(input: TokenStream) -> Result { let input: PyCompileInput = parse2(input)?; let args = input.parse(false)?; @@ -353,7 +353,7 @@ pub fn impl_py_compile(input: TokenStream2) -> Result Ok(output) } -pub fn impl_py_freeze(input: TokenStream2) -> Result { +pub fn impl_py_freeze(input: TokenStream) -> Result { let input: PyCompileInput = parse2(input)?; let args = input.parse(true)?; diff --git a/derive/src/error.rs b/derive/src/error.rs index 7884aa0c4..5ab6e5b28 100644 --- a/derive/src/error.rs +++ b/derive/src/error.rs @@ -33,7 +33,7 @@ use syn::parse::Error; macro_rules! err_span { ($span:expr, $($msg:tt)*) => ( - $crate::Diagnostic::spanned_error(&$span, format!($($msg)*)) + syn::Error::new_spanned(&$span, format!($($msg)*)) ) } @@ -84,16 +84,7 @@ impl Diagnostic { } } - pub fn span_error>(span: Span, text: T) -> Diagnostic { - Diagnostic { - inner: Repr::Single { - text: text.into(), - span: Some((span, span)), - }, - } - } - - pub fn spans_error>(spans: (Span, Span), text: T) -> Diagnostic { + pub(crate) fn spans_error>(spans: (Span, Span), text: T) -> Diagnostic { Diagnostic { inner: Repr::Single { text: text.into(), @@ -102,15 +93,6 @@ impl Diagnostic { } } - pub fn spanned_error>(node: &dyn ToTokens, text: T) -> Diagnostic { - Diagnostic { - inner: Repr::Single { - text: text.into(), - span: extract_spans(node), - }, - } - } - pub fn from_vec(diagnostics: Vec) -> Result<(), Diagnostic> { if diagnostics.is_empty() { Ok(()) diff --git a/derive/src/from_args.rs b/derive/src/from_args.rs index 82f1b2dc9..e307420db 100644 --- a/derive/src/from_args.rs +++ b/derive/src/from_args.rs @@ -1,8 +1,9 @@ use crate::util::path_eq; -use crate::Diagnostic; -use proc_macro2::TokenStream as TokenStream2; +use proc_macro2::TokenStream; use quote::{quote, ToTokens}; -use syn::{parse_quote, Attribute, Data, DeriveInput, Expr, Field, Ident, Lit, Meta, NestedMeta}; +use syn::{ + parse_quote, Attribute, Data, DeriveInput, Expr, Field, Ident, Lit, Meta, NestedMeta, Result, +}; /// The kind of the python parameter, this corresponds to the value of Parameter.kind /// (https://docs.python.org/3/library/inspect.html#inspect.Parameter.kind) @@ -34,7 +35,7 @@ struct ArgAttribute { type DefaultValue = Option; impl ArgAttribute { - fn from_attribute(attr: &Attribute) -> Option> { + fn from_attribute(attr: &Attribute) -> Option> { if !attr.path.is_ident("pyarg") { return None; } @@ -75,7 +76,7 @@ impl ArgAttribute { Some(inner()) } - fn parse_argument(&mut self, arg: &NestedMeta) -> Result<(), Diagnostic> { + fn parse_argument(&mut self, arg: &NestedMeta) -> Result<()> { if let ParameterKind::Flatten = self.kind { bail_span!(arg, "can't put additional arguments on a flatten arg") } @@ -119,12 +120,12 @@ impl ArgAttribute { } } -fn generate_field((i, field): (usize, &Field)) -> Result { +fn generate_field((i, field): (usize, &Field)) -> Result { let mut pyarg_attrs = field .attrs .iter() .filter_map(ArgAttribute::from_attribute) - .collect::, _>>()?; + .collect::, _>>()?; let attr = if pyarg_attrs.is_empty() { ArgAttribute { name: None, @@ -202,13 +203,13 @@ fn generate_field((i, field): (usize, &Field)) -> Result Result { +pub fn impl_from_args(input: DeriveInput) -> Result { let fields = match input.data { Data::Struct(syn::DataStruct { fields, .. }) => fields .iter() .enumerate() .map(generate_field) - .collect::>()?, + .collect::>()?, _ => bail_span!(input, "FromArgs input must be a struct"), }; diff --git a/derive/src/lib.rs b/derive/src/lib.rs index 156f7f1b8..2182f3e46 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -21,23 +21,28 @@ mod pystructseq; mod pyvalue; use error::{extract_spans, Diagnostic}; -use proc_macro::TokenStream; -use proc_macro2::TokenStream as TokenStream2; +use proc_macro2::TokenStream; use quote::ToTokens; use syn::{parse_macro_input, AttributeArgs, DeriveInput, Item}; -fn result_to_tokens(result: Result) -> TokenStream { - result.unwrap_or_else(ToTokens::into_token_stream).into() +fn result_to_tokens(result: Result>) -> proc_macro::TokenStream { + result + .map_err(|e| e.into()) + .unwrap_or_else(ToTokens::into_token_stream) + .into() } #[proc_macro_derive(FromArgs, attributes(pyarg))] -pub fn derive_from_args(input: TokenStream) -> TokenStream { +pub fn derive_from_args(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as DeriveInput); result_to_tokens(from_args::impl_from_args(input)) } #[proc_macro_attribute] -pub fn pyclass(attr: TokenStream, item: TokenStream) -> TokenStream { +pub fn pyclass( + attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { let attr = parse_macro_input!(attr as AttributeArgs); let item = parse_macro_input!(item as Item); result_to_tokens(pyclass::impl_pyclass(attr, item)) @@ -54,7 +59,7 @@ pub fn pyclass(attr: TokenStream, item: TokenStream) -> TokenStream { /// So, we use `extend_class!` macro as the second /// step in exception type definition. #[proc_macro] -pub fn define_exception(input: TokenStream) -> TokenStream { +pub fn define_exception(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let exc_def = parse_macro_input!(input as pyclass::PyExceptionDef); result_to_tokens(pyclass::impl_define_exception(exc_def)) } @@ -62,44 +67,53 @@ pub fn define_exception(input: TokenStream) -> TokenStream { /// Helper macro to define `Exception` types. /// More-or-less is an alias to `pyclass` macro. #[proc_macro_attribute] -pub fn pyexception(attr: TokenStream, item: TokenStream) -> TokenStream { +pub fn pyexception( + attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { let attr = parse_macro_input!(attr as AttributeArgs); let item = parse_macro_input!(item as Item); result_to_tokens(pyclass::impl_pyexception(attr, item)) } #[proc_macro_attribute] -pub fn pyimpl(attr: TokenStream, item: TokenStream) -> TokenStream { +pub fn pyimpl( + attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { let attr = parse_macro_input!(attr as AttributeArgs); let item = parse_macro_input!(item as Item); result_to_tokens(pyclass::impl_pyimpl(attr, item)) } #[proc_macro_attribute] -pub fn pymodule(attr: TokenStream, item: TokenStream) -> TokenStream { +pub fn pymodule( + attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { let attr = parse_macro_input!(attr as AttributeArgs); let item = parse_macro_input!(item as Item); result_to_tokens(pymodule::impl_pymodule(attr, item)) } #[proc_macro_derive(PyStructSequence)] -pub fn pystruct_sequence(input: TokenStream) -> TokenStream { +pub fn pystruct_sequence(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as DeriveInput); result_to_tokens(pystructseq::impl_pystruct_sequence(input)) } #[proc_macro] -pub fn py_compile(input: TokenStream) -> TokenStream { +pub fn py_compile(input: proc_macro::TokenStream) -> proc_macro::TokenStream { result_to_tokens(compile_bytecode::impl_py_compile(input.into())) } #[proc_macro] -pub fn py_freeze(input: TokenStream) -> TokenStream { +pub fn py_freeze(input: proc_macro::TokenStream) -> proc_macro::TokenStream { result_to_tokens(compile_bytecode::impl_py_freeze(input.into())) } #[proc_macro_derive(PyValue)] -pub fn pyvalue(input: TokenStream) -> TokenStream { +pub fn pyvalue(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as DeriveInput); result_to_tokens(pyvalue::impl_pyvalue(input)) } diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index 22ae3fa64..ec355cbaf 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -47,10 +47,7 @@ fn extract_items_into_context<'a, Item>( context.errors.ok_or_push(context.getset_items.validate()); } -pub(crate) fn impl_pyimpl( - attr: AttributeArgs, - item: Item, -) -> std::result::Result { +pub(crate) fn impl_pyimpl(attr: AttributeArgs, item: Item) -> Result { let mut context = ImplContext::default(); let mut tokens = match item { Item::Impl(mut imp) => { @@ -145,7 +142,7 @@ fn generate_class_def( base: Option, metaclass: Option, attrs: &[Attribute], -) -> std::result::Result { +) -> Result { let doc = attrs.doc().or_else(|| { let module_name = module_name.unwrap_or("builtins"); crate::doc::try_module_item(module_name, name) @@ -181,8 +178,7 @@ fn generate_class_def( return Err(syn::Error::new_spanned( ident, "PyStructSequence cannot have `base` class attr", - ) - .into()); + )); } let base_class = if is_pystruct { Some(quote! { rustpython_vm::builtins::PyTuple }) @@ -235,10 +231,7 @@ fn generate_class_def( Ok(tokens) } -pub(crate) fn impl_pyclass( - attr: AttributeArgs, - item: Item, -) -> std::result::Result { +pub(crate) fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result { let (ident, attrs) = pyclass_ident_and_attrs(&item)?; let fake_ident = Ident::new("pyclass", item.span()); let class_meta = ClassItemMeta::from_nested(ident.clone(), fake_ident, attr.into_iter())?; @@ -270,10 +263,7 @@ pub(crate) fn impl_pyclass( /// But, inside `macro_rules` we don't have an opportunity /// to add non-literal attributes to `pyclass`. /// That's why we have to use this proxy. -pub(crate) fn impl_pyexception( - attr: AttributeArgs, - item: Item, -) -> std::result::Result { +pub(crate) fn impl_pyexception(attr: AttributeArgs, item: Item) -> Result { let class_name = parse_vec_ident(&attr, &item, 0, "first 'class_name'")?; let base_class_name = parse_vec_ident(&attr, &item, 1, "second 'base_class_name'")?; @@ -292,9 +282,7 @@ pub(crate) fn impl_pyexception( Ok(ret) } -pub(crate) fn impl_define_exception( - exc_def: PyExceptionDef, -) -> std::result::Result { +pub(crate) fn impl_define_exception(exc_def: PyExceptionDef) -> Result { let PyExceptionDef { class_name, base_class, @@ -871,10 +859,7 @@ struct ExtractedImplAttrs { flags: TokenStream, } -fn extract_impl_attrs( - attr: AttributeArgs, - item: &Ident, -) -> std::result::Result { +fn extract_impl_attrs(attr: AttributeArgs, item: &Ident) -> Result { let mut withs = Vec::new(); let mut with_slots = Vec::new(); let mut flags = vec![quote! { ::rustpython_vm::slots::PyTypeFlags::DEFAULT.bits() }]; @@ -1090,7 +1075,7 @@ fn parse_vec_ident( item: &Item, index: usize, message: &str, -) -> std::result::Result { +) -> Result { Ok(attr .get(index) .ok_or_else(|| { diff --git a/derive/src/pymodule.rs b/derive/src/pymodule.rs index 58b297d6a..113c156d1 100644 --- a/derive/src/pymodule.rs +++ b/derive/src/pymodule.rs @@ -16,10 +16,7 @@ struct ModuleContext { errors: Vec, } -pub fn impl_pymodule( - attr: AttributeArgs, - module_item: Item, -) -> std::result::Result { +pub fn impl_pymodule(attr: AttributeArgs, module_item: Item) -> Result { let (doc, mut module_item) = match module_item { Item::Mod(m) => (m.attrs.doc(), m), other => bail_span!(other, "#[pymodule] can only be on a full module"), diff --git a/derive/src/pystructseq.rs b/derive/src/pystructseq.rs index 2c23f7252..fcd952472 100644 --- a/derive/src/pystructseq.rs +++ b/derive/src/pystructseq.rs @@ -1,11 +1,8 @@ -use super::Diagnostic; use proc_macro2::TokenStream; use quote::quote; -use syn::DeriveInput; +use syn::{DeriveInput, Result}; -pub(crate) fn impl_pystruct_sequence( - input: DeriveInput, -) -> std::result::Result { +pub(crate) fn impl_pystruct_sequence(input: DeriveInput) -> Result { let fields = if let syn::Data::Struct(ref struc) = input.data { &struc.fields } else { diff --git a/derive/src/pyvalue.rs b/derive/src/pyvalue.rs index fc9c7826e..64878f071 100644 --- a/derive/src/pyvalue.rs +++ b/derive/src/pyvalue.rs @@ -1,9 +1,8 @@ -use super::Diagnostic; use proc_macro2::TokenStream; use quote::quote; -use syn::DeriveInput; +use syn::{DeriveInput, Result}; -pub(crate) fn impl_pyvalue(input: DeriveInput) -> std::result::Result { +pub(crate) fn impl_pyvalue(input: DeriveInput) -> Result { let ty = &input.ident; let ret = quote! { From 7dd6891c6b2ca446fdb63e0b879c21fb7ce0d5a0 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 4 Oct 2021 13:49:38 +0900 Subject: [PATCH 2/2] clean up imports --- derive/src/compile_bytecode.rs | 19 ++++++++++++------- derive/src/pyclass.rs | 7 ++++--- derive/src/util.rs | 12 ++++++++---- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/derive/src/compile_bytecode.rs b/derive/src/compile_bytecode.rs index 9b361a516..3b9feedc6 100644 --- a/derive/src/compile_bytecode.rs +++ b/derive/src/compile_bytecode.rs @@ -19,13 +19,18 @@ use proc_macro2::{Span, TokenStream}; use quote::quote; use rustpython_bytecode::{CodeObject, FrozenModule}; use rustpython_compiler as compile; -use std::collections::HashMap; -use std::env; -use std::fs; -use std::path::{Path, PathBuf}; -use syn::parse::{Parse, ParseStream, Result as ParseResult}; -use syn::spanned::Spanned; -use syn::{self, parse2, Lit, LitByteStr, LitStr, Macro, Meta, MetaNameValue, Token}; +use std::{ + collections::HashMap, + env, fs, + path::{Path, PathBuf}, +}; +use syn::{ + self, + parse::{Parse, ParseStream, Result as ParseResult}, + parse2, + spanned::Spanned, + Lit, LitByteStr, LitStr, Macro, Meta, MetaNameValue, Token, +}; static CARGO_MANIFEST_DIR: Lazy = Lazy::new(|| { PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR is not present")) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index ec355cbaf..f8b08291a 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -6,10 +6,11 @@ use crate::util::{ use proc_macro2::TokenStream; use quote::{quote, quote_spanned, ToTokens}; use std::collections::HashMap; -use syn::parse::{Parse, ParseStream, Result as ParsingResult}; use syn::{ - parse_quote, spanned::Spanned, Attribute, AttributeArgs, Ident, Item, LitStr, Meta, NestedMeta, - Result, Token, + parse::{Parse, ParseStream, Result as ParsingResult}, + parse_quote, + spanned::Spanned, + Attribute, AttributeArgs, Ident, Item, LitStr, Meta, NestedMeta, Result, Token, }; use syn_ext::ext::*; diff --git a/derive/src/util.rs b/derive/src/util.rs index a30bac25c..ed790d810 100644 --- a/derive/src/util.rs +++ b/derive/src/util.rs @@ -2,10 +2,14 @@ use indexmap::map::IndexMap; use proc_macro2::{Span, TokenStream}; use quote::{quote, ToTokens}; use std::collections::HashMap; -use syn::Signature; -use syn::{spanned::Spanned, Attribute, Ident, Meta, MetaList, NestedMeta, Path, Result, UseTree}; -use syn_ext::ext::{AttributeExt as SynAttributeExt, *}; -use syn_ext::types::PunctuatedNestedMeta; +use syn::{ + spanned::Spanned, Attribute, Ident, Meta, MetaList, NestedMeta, Path, Result, Signature, + UseTree, +}; +use syn_ext::{ + ext::{AttributeExt as SynAttributeExt, *}, + types::PunctuatedNestedMeta, +}; pub(crate) const ALL_ALLOWED_NAMES: &[&str] = &[ "pymethod",