Skip to content

Newsyn #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 24, 2019
Merged

Newsyn #364

merged 1 commit into from
Aug 24, 2019

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Aug 3, 2019

r? @therealprof

New syn, quote and proc_macro2.

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Aug 3, 2019
@therealprof
Copy link
Contributor

Vec<TokenStream>

Nice cheat! I was actually trying to do the same but getting rid of the Vec in the same go.

@burrbull
Copy link
Member Author

burrbull commented Aug 3, 2019

Please, merge #362 .

@@ -213,24 +213,25 @@ pub fn fields(
} = f.bit_range;
let sc = f.name.to_sanitized_snake_case();
let pc = f.name.to_sanitized_upper_case();
let pc_r = Ident::from(&*format!("{}_A", pc));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why all the from -> new changes? I've introduced the from so the update wouldn't require so many changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't to make From on new syn to work.
Maybe some features disabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably. from should be the same as new but automatically using the current call site as span but maybe that has changed again in a newer version.

Copy link
Member Author

Choose a reason for hiding this comment

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

 the trait `std::convert::From<&str>` is not implemented for `proc_macro2::Ident`

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@therealprof therealprof Aug 3, 2019

Choose a reason for hiding this comment

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

proc_macro2::Ident?

Copy link
Member Author

Choose a reason for hiding this comment

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

syn reexport Ident from proc_macro2 now

Copy link
Contributor

Choose a reason for hiding this comment

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

Meh, this is all so weird.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, fine, let's go full on nuts with this. Will need proper testing though.

Copy link
Contributor

Choose a reason for hiding this comment

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

bors try

bors bot added a commit that referenced this pull request Aug 3, 2019
@bors
Copy link
Contributor

bors bot commented Aug 3, 2019

try

Build succeeded

@burrbull burrbull marked this pull request as ready for review August 4, 2019 03:20
@burrbull burrbull requested a review from a team as a code owner August 4, 2019 03:20
@therealprof therealprof mentioned this pull request Aug 4, 2019
@burrbull
Copy link
Member Author

burrbull commented Aug 6, 2019

This branch compiles in configuration with last syn:

quote-next = "1.0.0-rc1"
svd-parser = "0.7"
proc-macro2-next = "1.0.0-rc1"

[dependencies.syn-next]
version = "1.0.0-rc1"
features = ["full","extra-traits"]

So, I think PR can be merged.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Would you please add a CHANGELOG entry?

@burrbull
Copy link
Member Author

Rebased on master.

@burrbull
Copy link
Member Author

syn v1.0

@burrbull
Copy link
Member Author

@therealprof Is there something blocking merge?

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Sorry, was away for a few days. LGTM.

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Aug 24, 2019
364: Newsyn r=therealprof a=burrbull

r? @therealprof 

New `syn`, `quote` and `proc_macro2`.

Co-authored-by: Andrey Zgarbul <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 24, 2019

Build failed

@burrbull
Copy link
Member Author

bors retry r=therealprof

@bors
Copy link
Contributor

bors bot commented Aug 24, 2019

🔒 Permission denied

Existing reviewers: click here to make burrbull a reviewer

@therealprof
Copy link
Contributor

I've added you to bors so you should be able to try/retry now on this repository.

@burrbull
Copy link
Member Author

Thank you.

@burrbull
Copy link
Member Author

bors retry r=therealprof

bors bot added a commit that referenced this pull request Aug 24, 2019
364: Newsyn r=therealprof a=burrbull

r? @therealprof 

New `syn`, `quote` and `proc_macro2`.

Co-authored-by: Andrey Zgarbul <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 24, 2019

Build failed

@burrbull
Copy link
Member Author

bors retry r=therealprof

bors bot added a commit that referenced this pull request Aug 24, 2019
364: Newsyn r=therealprof a=burrbull

r? @therealprof 

New `syn`, `quote` and `proc_macro2`.

Co-authored-by: Andrey Zgarbul <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 24, 2019

Build succeeded

@bors bors bot merged commit 8759ff0 into rust-embedded:master Aug 24, 2019
@burrbull burrbull deleted the newsyn branch September 1, 2019 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants