Skip to content

Macro stuff#4687

Open
coolreader18 wants to merge 4 commits into
RustPython:mainfrom
coolreader18:macro-stuff
Open

Macro stuff#4687
coolreader18 wants to merge 4 commits into
RustPython:mainfrom
coolreader18:macro-stuff

Conversation

@coolreader18
Copy link
Copy Markdown
Member

This is sorta a work in progress that's ready for review, mainly since I'm still not sure whether parallelizing py_freeze! is actually a good idea. My original idea was to just output py_compile! invocations from freeze and let rustc parallelize it for us, but it turns out it doesn't actually run proc macros in parallel, so.

@youknowone
Copy link
Copy Markdown
Member

@coolreader18 I like this patch so much even without parallel parts. Could you split this PR to 2 chunks one for refactoring and the other one for parallel compilation?

Comment thread derive-impl/src/compile_bytecode.rs Outdated
let path = input.call(syn::Path::parse_mod_style)?;
let eq_token: Token![=] = input.parse()?;
let span = input.span();
fn parse_litstr(input: ParseStream) -> ParseResult<LitStr> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn parse_litstr(input: ParseStream) -> ParseResult<LitStr> {
fn parse_lit_str(input: ParseStream) -> ParseResult<LitStr> {

Comment thread derive-impl/src/compile_bytecode.rs Outdated
}
}

impl Parse for PyCompileArgs {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the motivation to this change?

Comment thread derive-impl/src/util.rs Outdated
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
Ord::cmp(&self.name, &other.name).then_with(|| {
cmp_iters(self.cfgs.iter(), other.cfgs.iter(), |a, b| {
crate::util::cmp_tokenstream(a.to_token_stream(), b.to_token_stream())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
crate::util::cmp_tokenstream(a.to_token_stream(), b.to_token_stream())
cmp_tokenstream(a.to_token_stream(), b.to_token_stream())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we get string from tokenstream and compare them? this is looking a bit too much.

Comment on lines +132 to +133
} else {
return Err(e);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
return Err(e);

@coolreader18
Copy link
Copy Markdown
Member Author

Ough now I wanna use syn 2's meta parsing api instead but I'd have to update like syn-ext and stuff too

@youknowone
Copy link
Copy Markdown
Member

now we use syn2. will this patch be revived?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants