save-analysis: use a decoupled representation for dumped data #33348
Comments
|
cc @nrc Do you know the answer to my questions in the open problems part? |
|
@aochagavia if the def id is from the local crate, then it should never return None. I think we can assume that and just unwrap. We do need to make sure we only call with local ids though, it is possible we could get a NodeId from an extern crate (via metadata or something?). |
Manishearth
added a commit
to Manishearth/rust
that referenced
this issue
May 9, 2016
save-analysis: use a decoupled representation for dumped data Closes rust-lang#33348 This will probably break any tool relying on the csv backend of save_analysis, for the following reasons: 1. Dumped spans don't contain extents anymore (`Dump` uses `SpanData` now instead of internal `Span`s). In case we still want to dump extents we could add them to `SpanData`. 1. `DefId`s are no longer dumped as a pair of `(ref_id, ref_crate)`. Instead, they are dumped as a single `Id`. @nrc You said something about storing the id in a `u64`, but you didn't explain why. I kept using `u32` in this branch but I can change it if you prefer that. r? @nrc By the way, the fact that this breaks tools relying on CSV may be a good occasion to start dumping CSV in a different way (i.e. using the serializer like in the JSON backend).
Manishearth
added a commit
to Manishearth/rust
that referenced
this issue
May 9, 2016
save-analysis: use a decoupled representation for dumped data Closes rust-lang#33348 This will probably break any tool relying on the csv backend of save_analysis, for the following reasons: 1. Dumped spans don't contain extents anymore (`Dump` uses `SpanData` now instead of internal `Span`s). In case we still want to dump extents we could add them to `SpanData`. 1. `DefId`s are no longer dumped as a pair of `(ref_id, ref_crate)`. Instead, they are dumped as a single `Id`. @nrc You said something about storing the id in a `u64`, but you didn't explain why. I kept using `u32` in this branch but I can change it if you prefer that. r? @nrc By the way, the fact that this breaks tools relying on CSV may be a good occasion to start dumping CSV in a different way (i.e. using the serializer like in the JSON backend).
Manishearth
added a commit
to Manishearth/rust
that referenced
this issue
May 9, 2016
save-analysis: use a decoupled representation for dumped data Closes rust-lang#33348 This will probably break any tool relying on the csv backend of save_analysis, for the following reasons: 1. Dumped spans don't contain extents anymore (`Dump` uses `SpanData` now instead of internal `Span`s). In case we still want to dump extents we could add them to `SpanData`. 1. `DefId`s are no longer dumped as a pair of `(ref_id, ref_crate)`. Instead, they are dumped as a single `Id`. @nrc You said something about storing the id in a `u64`, but you didn't explain why. I kept using `u32` in this branch but I can change it if you prefer that. r? @nrc By the way, the fact that this breaks tools relying on CSV may be a good occasion to start dumping CSV in a different way (i.e. using the serializer like in the JSON backend).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, the
Dumptrait exposes internal types of the compiler, which is undesirable. It also forces the trait's implementors to write lowering code (as can be seen in the current implementation ofjson_dumper).Proposed solution
save-analysiscalledexternal_datawhere the new*Datastructs will live (they can be moved fromjson_dumper, along with the lowering code).Dumptrait to operate onsave_analysis::external_data::*instead of onsave_analysis::data::*.Some open problems
When getting a
DefIdfrom aNodeId, what should we do whentcx.map.opt_local_def_id(node_id)returnsNone? What does that mean? If we call the function with aNodeIdprovided by the compiler, should it always succeed? The current code doesn't make that assumption (see data.rs)The text was updated successfully, but these errors were encountered: