Remove unnecessary mutable borrow and resizing in DepGraph::serialize #50598
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
Thanks for the PR, @whitfin! For a long time we had the invariant that the fingerprints vec has to be of the same length as the dep-node vec because you can map from dep-node to fingerprint. The exception are so-called "anonymous" dep-nodes which don't have a corresponding fingerprint. This is why the size difference can arise in the first place. At this point, with current state of the code, the size difference should not actually cause any trouble anymore though. Let's give it a try! @bors r+ |
|
📌 Commit ae3feff has been approved by |
bors
added
S-waiting-on-bors
…ichaelwoerister Remove unnecessary mutable borrow and resizing in DepGraph::serialize I might be mistaken, but I noticed this whilst in this file for something else. It appears that this mutable borrow is unnecessary and since it's locking it should be removed. The resizing looks redundant since nothing additional is added to the fingerprints in this function, so that can also be removed.
…ichaelwoerister Remove unnecessary mutable borrow and resizing in DepGraph::serialize I might be mistaken, but I noticed this whilst in this file for something else. It appears that this mutable borrow is unnecessary and since it's locking it should be removed. The resizing looks redundant since nothing additional is added to the fingerprints in this function, so that can also be removed.
Rollup of 18 pull requests Successful merges: - #49423 (Extend tests for RFC1598 (GAT)) - #50010 (Give SliceIndex impls a test suite of girth befitting the implementation (and fix a UTF8 boundary check)) - #50447 (Fix update-references for tests within subdirectories.) - #50514 (Pull in a wasm fix from LLVM upstream) - #50524 (Make DepGraph::previous_work_products immutable) - #50532 (Don't use Lock for heavily accessed CrateMetadata::cnum_map.) - #50538 ( Make CrateNum allocation more thread-safe. ) - #50564 (Inline `Span` methods.) - #50565 (Use SmallVec for DepNodeIndex within dep_graph.) - #50569 (Allow for specifying a linker plugin for cross-language LTO) - #50572 (Clarify in the docs that `mul_add` is not always faster.) - #50574 (add fn `into_inner(self) -> (Idx, Idx)` to RangeInclusive (#49022)) - #50575 (std: Avoid `ptr::copy` if unnecessary in `vec::Drain`) - #50588 (Move "See also" disambiguation links for primitive types to top) - #50590 (Fix tuple struct field spans) - #50591 (Restore RawVec::reserve* documentation) - #50598 (Remove unnecessary mutable borrow and resizing in DepGraph::serialize) - #50606 (Retry when downloading the Docker cache.) Failed merges: - #50161 (added missing implementation hint) - #50558 (Remove all reference to DepGraph::work_products)
I might be mistaken, but I noticed this whilst in this file for something else. It appears that this mutable borrow is unnecessary and since it's locking it should be removed. The resizing looks redundant since nothing additional is added to the fingerprints in this function, so that can also be removed.