Skip to content
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

internal: Use upstream exhaustiveness checker! #16420

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

Nadrieril
Copy link
Member

Because it has been librarified!

The extra Apache-2.0 WITH LLVM-exception license is for rustc_apfloat. Also this duplicates rustc_index because the other upstream deps are still on an earlier version. They should be bumpable now though. Good thing is that we don't need this new crate to be synchronized with the others, which will make our lives easier.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 23, 2024
crates/hir-ty/src/lib.rs Outdated Show resolved Hide resolved
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6c4085e0c771fd4b883930b599ef42966b855762bbe4052c17673b3253421a6d"
dependencies = [
"derivative",
Copy link
Member

Choose a reason for hiding this comment

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

Oof, we went through some pains to get rid of syn 1.0. But you do seem to use it in a couple of places, so doing that by hand would probably be too annoying.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh no 🙈 I mean it's not too hard to impl by hand

Copy link
Member

Choose a reason for hiding this comment

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

It's not, but I don't want to force you to spend time on that right now. It's okay to merge, then clean it up later (ideally).

Copy link
Member

Choose a reason for hiding this comment

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

We should just fix up the assist to inline custom proc-macro derive expansions (which currently only supports builtin derives) :)

But to chime in as well, I'd like to see derivative not stick in our dependency tree for long. Iirc it, like syn, has a pretty bad compile time impact.

It's okay to merge, then clean it up later (ideally).

☝️

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe mcarton/rust-derivative#118 can save us! If it ever gets merged

Copy link
Member Author

@Nadrieril Nadrieril Jan 24, 2024

Choose a reason for hiding this comment

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

It seems rustc also wants to get rid of syn 1.0 rust-lang/rust#109302, and they use derivative. That's a good sign

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, actually rust-derivative does not seem to be maintained anymore (3 years since the last commit). Anyways, since rustc wants to bump to syn 2 as well T-compiler will probably figure something out there at some point.

Copy link
Member Author

@Nadrieril Nadrieril Jan 27, 2024

Choose a reason for hiding this comment

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

Yeah, that PR I linked was opened recently by the person who's working on moving rustc to syn2, hopefully they'll take some kind of action if the maintainer doesn't respond

Copy link
Member

Choose a reason for hiding this comment

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

Fine, I'll do it 😂.

@Veykril
Copy link
Member

Veykril commented Jan 24, 2024

Excited to see what we can do with this!
@bors r+

@bors
Copy link
Collaborator

bors commented Jan 24, 2024

📌 Commit 2370b70 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jan 24, 2024

⌛ Testing commit 2370b70 with merge 0d52934...

@bors
Copy link
Collaborator

bors commented Jan 24, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 0d52934 to master...

@bors bors merged commit 0d52934 into rust-lang:master Jan 24, 2024
10 checks passed
@Nadrieril Nadrieril deleted the use-upstream-pattern-analysis branch January 24, 2024 15:05
@lnicola lnicola changed the title Use upstream exhaustiveness checker! internal: Use upstream exhaustiveness checker! Jan 27, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 27, 2024
…tive, r=Nilstrieb

Stop using derivative in rustc_pattern_analysis

CC rust-lang#109302, rust-lang/rust-analyzer#16420 (comment)

r? `@Nadrieril`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 27, 2024
…tive, r=Nilstrieb

Stop using derivative in rustc_pattern_analysis

CC rust-lang#109302, rust-lang/rust-analyzer#16420 (comment)

r? `@Nadrieril`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 27, 2024
…tive, r=Nilstrieb

Stop using derivative in rustc_pattern_analysis

CC rust-lang#109302, rust-lang/rust-analyzer#16420 (comment)

r? ``@Nadrieril``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 28, 2024
…tive, r=Nilstrieb

Stop using derivative in rustc_pattern_analysis

CC rust-lang#109302, rust-lang/rust-analyzer#16420 (comment)

r? ```@Nadrieril```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jan 29, 2024
…tive, r=Nilstrieb

Stop using derivative in rustc_pattern_analysis

CC rust-lang#109302, rust-lang/rust-analyzer#16420 (comment)

r? ````@Nadrieril````
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2024
Rollup merge of rust-lang#120420 - lnicola:rm-pattern-analysis-derivative, r=Nilstrieb

Stop using derivative in rustc_pattern_analysis

CC rust-lang#109302, rust-lang/rust-analyzer#16420 (comment)

r? ````@Nadrieril````
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants