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: Add RA_UNSTABLE_SYSROOT_HACK #14599

Merged
merged 1 commit into from
Apr 20, 2023
Merged

internal: Add RA_UNSTABLE_SYSROOT_HACK #14599

merged 1 commit into from
Apr 20, 2023

Conversation

HKalbasi
Copy link
Member

cc #7637

It is currently in a proof of concept stage, and it doesn't generates a copy. You need to provide your own sysroot copy in /tmp/ra-sysroot-hack in a way that /tmp/ra-sysroot-hack/library/std/lib.rs exists and /tmp/ra-sysroot-hack/Cargo.toml is the one from this comment. I will add the symlink code if we decide that this approach is not a dead end.

It seems to somehow work on my system. Go to definition into std dependencies works, type checking can look through fields if I make them public and cfg_if appears to work (I tested it by hovering both sides and seeing that the correct one is enabled). Though finding layout of HashMap didn't work.

Please try it and let me know if I should go forward in this direction or not.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2023
@HKalbasi
Copy link
Member Author

The reason that layout of HashMap doesn't work is not resolving core things that hashbrown uses. If I replace it in hashbrown with an i32, layout works, but if I replace fields with Option<i32> it doesn't work. It is strange to me since go to definition from hashbrown to core works.

@Veykril
Copy link
Member

Veykril commented Apr 18, 2023

Dependencies of std require weird workarounds due to them not being allowed to depend on the std lib directly themselves, wouldn't surprise me if r-a fails to understand those workarounds somehow

@HKalbasi
Copy link
Member Author

Yes that was it. I thought it should work out of the box and the fact that go to definition worked from hashbrown to core misled me that it is working. I manually replaced rustc_workspace_std_* with * in the crate graph and now it shows that size of HashMap<i32, i32> is 48 bytes.

Should I continue this path and add the symlink code?

@Veykril
Copy link
Member

Veykril commented Apr 19, 2023

Given cargo was made its own workspace in the rust-lang/rust repo now, I'd like to poke whether its feasible to pursue making library its own workspace once more in rust-lang/rust. (I am not too happy with the alternative approaches we have really, they feel too hacky and brittle)

@HKalbasi
Copy link
Member Author

Fair. Is it possible to merge this under a config flag, so that I can use it on my system? I want to work on the interpreting mir for unit tests, and my process is very "not-working driven development" :). That is, I see for example println!("hello") is not working, find out why, and fix the bug or implement the missing thing. The problem of this process is that problems are sequential, and next level is locked until you win the current level. And this issue is currently everywhere and blocked my further progress. By merging this I can continue to work and hopefully when I become done, this issue is already solved with a correct and non hacky solution and interpreting will become available for the general public.

@HKalbasi HKalbasi force-pushed the dev2 branch 2 times, most recently from 0b9e2dd to ab3f01b Compare April 19, 2023 11:26
@Veykril
Copy link
Member

Veykril commented Apr 19, 2023

I'll take a look at this later today or tomorrow (only skimmed it very briefly just now but from a rough look the changes seem quite invasive for a temporary hack), adding this as a temporary hack for you to continue developing sounds fine to me otherwise.

crates/project-model/src/workspace.rs Outdated Show resolved Hide resolved
crates/project-model/src/workspace.rs Outdated Show resolved Hide resolved
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

That's a lot less invasive 👍 thanks for changing it

@HKalbasi
Copy link
Member Author

Thanks for accepting it.

I still think it would be good enough to consider as a plan B if making std a cargo workspace didn't progressed. Specially after I used it and found its potential problems.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 20, 2023

📌 Commit 39715ce has been approved by HKalbasi

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 20, 2023

⌛ Testing commit 39715ce with merge 0289dfa...

@HKalbasi HKalbasi changed the title Detect sysroot dependencies using symlink copy internal: Add RA_UNSTABLE_SYSROOT_HACK Apr 20, 2023
@bors
Copy link
Contributor

bors commented Apr 20, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 0289dfa to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Apr 20, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 0289dfa to master...

@bors bors merged commit 0289dfa into rust-lang:master Apr 20, 2023
@bors
Copy link
Contributor

bors commented Apr 20, 2023

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

@HKalbasi
Copy link
Member Author

It seems this doesn't see the module files in libc crate, so for example shows the libc::unix::c_void as unresolved. But if I open the module file manually in vscode, it becomes resolved. The modules in libc don't use #[path = ".."] and other crates like hashbrown don't have this problem. Any idea why this is happening?

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.

4 participants