Remove forked rust dependency#19
Merged
Merged
Conversation
Collaborator
|
Going to test the integration with |
Collaborator
|
The integration tests in There are some problems with what becomes
There may be other changes in the json, these are the ones making the integration test fail. |
Collaborator
|
Another issue is this panic in the compiler. Could this be an unresolved issue in Stack trace here: DetailsThe call to |
Contributor
Author
|
Things are pretty fragile the moment. Sorry for that. For context, there were several competing forces that contributed to the current state of affairs:
Regarding the JSON output format breaking, it should be possible to the massage the output to what we had before --- I am not sure I will have time for that though. The second issue about the compiler error --- the bug is likely in our code --- many of these compiler queries (in this case, the misbehaving query is I hope this is helpful. |
Collaborator
|
This is helpful @sskeirik. @jberthold I will look into it when I can with you and we will figure out what we need to get mir-semantics accepting the current state, and document what needs to be improved of course |
This PR adds functionality to filter and normalise the json output when comparing output to the golden tests. Some cases when we want to normalise output: 1. `"NormalSym"` mangled function names will have a hash following `17h` that will [change when the compiler version changes](https://github.com/rust-lang/rust/blob/2ae9916816a448fcaab3b2da461de754eda0055a/compiler/rustc_symbol_mangling/src/lib.rs#L69-L76). ``` < "NormalSym": "_ZN3std2rt19lang_start_internal17h3166ab4c34d1da61E" --- > "NormalSym": "_ZN3std2rt19lang_start_internal17h51b943990c0e2c96E" ^----can ignore---^ ``` 2. `"symbol_name"` mangled function names will have a hash following `17h` that will [change when the compiler version changes](https://github.com/rust-lang/rust/blob/2ae9916816a448fcaab3b2da461de754eda0055a/compiler/rustc_symbol_mangling/src/lib.rs#L69-L76). ``` < "symbol_name": "_ZN4core3ops8function6FnOnce9call_once17h40ba616aaa505a69E" --- > "symbol_name": "_ZN4core3ops8function6FnOnce9call_once17hd98db8d17b807164E" ```
jberthold
approved these changes
Jan 16, 2025
dkcumming
approved these changes
Jan 16, 2025
dkcumming
left a comment
dkcumming
left a comment
Collaborator
There was a problem hiding this comment.
I think this is good enough to merge and get started on the next phase. I think there are some problems to solve, but we can focus on that after a push to get things all connected from start to finish.
Comment on lines
+704
to
+714
| // FIXME: We dump extra static items here --- this should be handled better | ||
| for (_, alloc) in visited_allocs.iter() { | ||
| if let AllocInfo::Static(def) = alloc { | ||
| let mono_item = stable_mir::mir::mono::MonoItem::Fn(stable_mir::mir::mono::Instance::from(*def)); | ||
| let item_name = &mono_item_name(tcx, &mono_item); | ||
| if !items_clone.contains_key(item_name) { | ||
| println!("Items missing static with id {:?} and name {:?}", def, item_name); | ||
| items.push(mk_item(tcx, mono_item, item_name.clone())); | ||
| } | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
Going to make an issue for this, so that we are tracking peculiarities about the format
This was referenced Jan 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes the forked rust dependency by implementing the following two functions directly in smir_pretty (instead of in a forked rust compiler):
This is done by extending the functionality of
LinkNameCollectorto also collectTyandAllocIdmappings and renaming the combined collector toInternedValueCollector.EDIT: When running the panic example on rustc master, there is some weirdness where a static item called:
alloc::alloc::__rust_no_alloc_shim_is_unstableis generated which has no body. This static item appears to be referenced from several different allocations (can be seen by chasing pointers in allocations provenance table). If this funny static item is findable on other machines, we should document this issue. It may well be that this is an empty type that is never used at runtime, but I haven't gone in to check this yet.