-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix Alias intra doc ICE #52835
Fix Alias intra doc ICE #52835
Conversation
956299e
to
bd7aea4
Compare
src/librustdoc/clean/mod.rs
Outdated
.find(|item| item.ident.name == item_name) | ||
let elem = match cx.tcx.type_of(did).sty { | ||
ty::TyAdt(def, _) => { | ||
if is_enum { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't even need to switch on is_enum
here: def.all_fields()
should work in general.
src/librustdoc/clean/mod.rs
Outdated
.fields | ||
.iter() | ||
.find(|item| item.ident.name == item_name) | ||
let elem = match cx.tcx.type_of(did).sty { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of storing elem
and then matching on it, you might as well subsume the if-else below into this match, because you only return Ok
if it's a TyAdt
.
You can also then use def.is_enum()
directly instead of matching on ty.def
.
r=me after the minor comments are addressed. |
bd7aea4
to
d5f1f70
Compare
Updated. |
@bors: r=varkor |
📌 Commit d5f1f70 has been approved by |
…=varkor Fix Alias intra doc ICE Fixes rust-lang#52611. cc @QuietMisdreavus r? @varkor
src/librustdoc/clean/mod.rs
Outdated
Ok((ty.def, | ||
Some(format!("{}.{}", | ||
if def.is_enum() { | ||
"variant" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think this is correct, it won't find variants... unless you want fields? This is confusing. Although, not any more wrong than before I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we need the differentiation between .non_enum_variant()
and .all_fields()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that in this code (and before), you're iterating over the fields (of a struct, enum, etc.), but if it's an enum, you're calling the result "variant"
(when it's more accurately an "enumfield"
or something). The question is whether you actually want fields or variants for enums. If you want fields, you should rename the string, but if you actually are looking for variants, you should be iterating over variants
if it's an enum, rather than all_fields()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what exactly you're looking for here, tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the type has a field or a variant with a given name.
@bors: r- |
Updated. |
@bors r+ (the |
Seems like bors doesn't like comments? @bors: r=eddyb |
📌 Commit d94bdf8 has been approved by |
…=eddyb Fix Alias intra doc ICE Fixes rust-lang#52611. cc @QuietMisdreavus r? @varkor
src/librustdoc/clean/mod.rs
Outdated
if let Some(item) = def.all_fields() | ||
.find(|item| item.ident.name == item_name) { | ||
if let Some(item) = if def.is_enum() { | ||
def.all_fields().find(|item| item.ident.name == item_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a change that needed to be reverted, unless you wanted to iterate through def.variants
here instead of def.all_fields()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes indeed!
…=eddyb Fix Alias intra doc ICE Fixes rust-lang#52611. cc @QuietMisdreavus r? @varkor
Rollup of 31 pull requests Successful merges: - #52332 (dead-code lint: say "constructed" for structs) - #52340 (Document From trait implementations for OsStr, OsString, CString, and CStr) - #52628 (Cleanup some rustdoc code) - #52732 (Remove unstable and deprecated APIs) - #52745 (Update clippy to latest master) - #52756 (rustc: Disallow machine applicability in foreign macros) - #52771 (Clarify thread::park semantics) - #52810 ([NLL] Don't make "fake" match variables mutable) - #52821 (pretty print for std::collections::vecdeque) - #52822 (Fix From<LocalWaker>) - #52824 (Fix -Wpessimizing-move warnings in rustllvm/PassWrapper) - #52831 (remove references to AUTHORS.txt file) - #52835 (Fix Alias intra doc ICE) - #52842 (update comment) - #52846 (Add timeout to use of `curl` in bootstrap.py.) - #52851 (Make the tool_lints actually usable) - #52853 (Improve bootstrap help on stages) - #52859 (Use Vec::extend in SmallVec::extend when applicable) - #52861 (Add targets for HermitCore (https://hermitcore.org) to the Rust compiler and port libstd to it.) - #52867 (releases.md: fix 2 typos) - #52870 (Implement Unpin for FutureObj and LocalFutureObj) - #52876 (run-pass/const-endianness: negate before to_le()) - #52878 (Fix wrong issue number in the test name) - #52883 (Include lifetime in mutability suggestion in NLL messages) - #52904 (NLL: sort diagnostics by span) - #52905 (Fix a typo in unsize.rs) - #52907 (NLL: On "cannot move out of type" error, print original before rewrite) - #52908 (Use SetLenOnDrop in Vec::truncate()) - #52914 (Only run the sparc-abi test on sparc) - #52918 (Backport 1.27.2 release notes) - #52929 (Update compatibility note for 1.28.0 to be correct) Failed merges: - #52758 (Cleanup for librustc::session) - #52799 (Use BitVector for global sets of AttrId) r? @ghost
…=eddyb Fix Alias intra doc ICE Fixes rust-lang#52611. cc @QuietMisdreavus r? @varkor
…=eddyb Fix Alias intra doc ICE Fixes rust-lang#52611. cc @QuietMisdreavus r? @varkor
Rollup of 15 pull requests Successful merges: - #52793 (Add test for NLL: unexpected "free region `` does not outlive" error ) - #52799 (Use BitVector for global sets of AttrId) - #52809 (Add test for unexpected region for local data ReStatic) - #52834 ([NLL] Allow conflicting borrows of promoted length zero arrays) - #52835 (Fix Alias intra doc ICE) - #52854 (fix memrchr in miri) - #52899 (tests/ui: Add missing mips{64} ignores) - #52908 (Use SetLenOnDrop in Vec::truncate()) - #52915 (Don't count MIR locals as borrowed after StorageDead when finding locals live across a yield terminator) - #52926 (rustc: Trim down the `rust_2018_idioms` lint group) - #52930 (rustc_resolve: record single-segment extern crate import resolutions.) - #52939 (Make io::Read::read_to_end consider io::Take::limit) - #52942 (Another SmallVec.extend optimization) - #52947 (1.27 actually added the `armv5te-unknown-linux-musleabi` target) - #52954 (async can begin expressions) Failed merges: r? @ghost
Fixes #52611.
cc @QuietMisdreavus
r? @varkor