Skip to content

Use unnamed consts in chalk-derive #717

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 12, 2021
Merged

Use unnamed consts in chalk-derive #717

merged 1 commit into from
Jul 12, 2021

Conversation

flodiebold
Copy link
Member

This makes the derives work in rust-analyzer.

CC rust-lang/rust-analyzer#9562

@lnicola
Copy link
Member

lnicola commented Jul 11, 2021

What about derive_visit and derive_super_visit? But yeah, I got the same error 😬.

@flodiebold
Copy link
Member Author

flodiebold commented Jul 11, 2021 •
edited
Loading

derive_visit and derive_super_visit both call derive_any_visit.

😕 no idea what's going on with these errors, the generated code looks fine...

Edit: Ah no, it's missing a semicolon in the end. How is this supposed to work?

@lnicola
Copy link
Member

lnicola commented Jul 11, 2021

But there's a semicolon at the end of https://github.com/mystor/synstructure/pull/43/files.

@flodiebold
Copy link
Member Author

flodiebold commented Jul 11, 2021 •
edited
Loading

Not in the one further up, there seem to be two versions. That would explain why it worked for the original author. One is bound_impl, the other is gen_impl, which apparently obsoletes the former.

lnicola added a commit to lnicola/synstructure that referenced this pull request Jul 11, 2021
@lnicola
Add missing semicolon for underscore_const
0f5e094 
See rust-lang/chalk#717
@lnicola lnicola mentioned this pull request Jul 11, 2021
Merged
@flodiebold
Copy link
Member Author

I've tried rewriting the derives to gen_impl, but it's rather annoying to do.

@lnicola
Copy link
Member

lnicola commented Jul 11, 2021

Yeah, it seems so. I files a PR against synstructure in the meanwhile, but it might take a while to get merged and published.

Maybe we should also work around this on our side?

@flodiebold flodiebold mentioned this pull request Jul 11, 2021
Merged
@flodiebold
Copy link
Member Author

I made a PR for my hacky workaround 🤷

mystor pushed a commit to mystor/synstructure that referenced this pull request Jul 11, 2021
@lnicola
Add missing semicolon for underscore_const (#47)
9dcacf2 
* Add missing semicolon for `underscore_const`

See rust-lang/chalk#717

* Fix unpretty test

* Bump version
@flodiebold
Use unnamed consts in chalk-derive
fecee2b 
This makes the derives work in rust-analyzer.

CC rust-lang/rust-analyzer#9562
@flodiebold
Copy link
Member Author

New synstructure version got released, so it works now 🙂

@lnicola
Copy link
Member

lnicola commented Jul 12, 2021

@shepmaster ugh

@lqd
Copy link
Member

lqd commented Jul 12, 2021 •
edited
Loading

This looks like github being confused about which PR in which fork actually caused this issue to be closed, it was because of Florian's "until the proper fix $this-issue" message would close this issue when the PR was merged in r-a.
image

Notice the underlined "fix" in bors merge commit
image

@lnicola
Copy link
Member

lnicola commented Jul 12, 2021

Yep. Surprisingly, though, GitHub didn't close this when bors merged the original PR in the RA repo.

@lqd
Copy link
Member

lqd commented Jul 12, 2021

Very confusing indeed

@shepmaster
Copy link
Member

Very odd!

My workflow involves pulling from my upstream (rust-analyzer/rust-analyzer) and pushing to my origin (shepmaster/rust-analyzer). I've never had this behavior happen before.

@shepmaster
Copy link
Member

I've also never seen it cross repositories before (rust-analyzer -> chalk), which is doubly strange.

@flodiebold flodiebold reopened this Jul 12, 2021
@jackh726
Copy link
Member

Not sure what the problem is here, but sure. @bors r+

@bors
Copy link
Contributor

bors commented Jul 12, 2021

📌 Commit fecee2b has been approved by jackh726

@bors
Copy link
Contributor

bors commented Jul 12, 2021

⌛ Testing commit fecee2b with merge eb24af1...

@lnicola
Copy link
Member

lnicola commented Jul 12, 2021

Not sure what the problem is here, but sure.

rust-analyzer only looks into unnamed consts for trait implementations, to speed up the analysis.

@bors
Copy link
Contributor

bors commented Jul 12, 2021

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing eb24af1 to master...

@bors bors merged commit eb24af1 into rust-lang:master Jul 12, 2021
@flodiebold flodiebold deleted the underscore-const branch July 12, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@flodiebold @lnicola @lqd @shepmaster @jackh726 @bors