Skip to content

use require_lang_item over unwrap. - #72170

Merged
bors merged 2 commits into
rust-lang:masterfrom
lcnr:lang_item
May 14, 2020
Merged

use require_lang_item over unwrap.#72170
bors merged 2 commits into
rust-lang:masterfrom
lcnr:lang_item

Conversation

@lcnr

@lcnrlcnr commented May 13, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Does not yet replace all uses of lang_items\(\)\.*\.unwrap\(\), as there are more
than I expected 😅

Fixes#72099

r? @RalfJung

edit: The goal of this this PR is to change ICE from missing lang items to a fatal error.

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2020
RalfJung
RalfJung reviewed May 13, 2020
View reviewed changes
Comment threadsrc/librustc_mir/borrow_check/type_check/mod.rs Outdated
RalfJung
RalfJung reviewed May 13, 2020
View reviewed changes
Comment threadsrc/librustc_mir/transform/generator.rs
RalfJung
RalfJung reviewed May 13, 2020
View reviewed changes
Comment threadsrc/librustc_mir/transform/generator.rs Outdated
@RalfJung

RalfJung commented May 13, 2020

Copy link
Copy Markdown
Member

Could you also cover the one that the issue was reported about?

rust/src/librustc_typeck/check/demand.rs

Line 637 in 3fe4dd2

let deref_trait = self.tcx.lang_items().deref_trait().unwrap();

@lcnr

lcnr commented May 13, 2020

Copy link
Copy Markdown
ContributorAuthor

done

RalfJung
RalfJung approved these changes May 14, 2020
View reviewed changes

@RalfJungRalfJung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This LGTM. However, this is code that I am not familiar with, so I'd prefer if someone else also took a look. Cc @oli-obk@matthewjasper

@oli-obk

oli-obk commented May 14, 2020

Copy link
Copy Markdown
Contributor

Oh, that's a very good idea. I wonder if we can make lang_items private at some point and just have sensible wrappers like require_lang_item.

But we can address this in future work. For now:

@bors r+

@bors

bors commented May 14, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 9001a64 has been approved by oli-obk

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2020
@jonas-schievink

jonas-schievink commented May 14, 2020

Copy link
Copy Markdown
Contributor

This looks like a good E-easy contribution, so maybe open an issue for the follow-up work here?

@lcnrlcnr mentioned this pull request May 14, 2020
Closed
@lcnr

lcnr commented May 14, 2020

Copy link
Copy Markdown
ContributorAuthor

@jonas-schievink Opened #72195 for this, not
sure what other labels apply there.

@Dylan-DPC-zzDylan-DPC-zz mentioned this pull request May 14, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 14, 2020
@bors
Auto merge ofrust-lang#72202- Dylan-DPC:rollup-6lbxh1s, r=Dylan-DPC
a74d186
Rollup of 8 pull requests
Successful merges:
- rust-lang#71910 (Fix unused_parens false positive when using binary operations)
- rust-lang#72087 (Fix hang in lexical_region_resolve)
- rust-lang#72126 (Change `WorkProduct::saved_files` to an `Option`.)
- rust-lang#72127 (add long error explanation for E0228)
- rust-lang#72141 (Warn against thread::sleep in async fn)
- rust-lang#72170 (use `require_lang_item` over `unwrap`.)
- rust-lang#72191 (Clean up E0589 explanation)
- rust-lang#72194 (Don't ICE on missing `Unsize` impl)
Failed merges:
r? @ghost
@cofibrantcofibrant mentioned this pull request May 14, 2020
Merged
@bors
bors merged commit 62f1840 into rust-lang:masterMay 14, 2020
@lcnr
lcnr deleted the lang_item branch May 15, 2020 11:01
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 15, 2020
@Dylan-DPC
Rollup merge ofrust-lang#72216- doctorn:require-lang-item, r=lcnr
af423c9
Remove `lang_items\(\).*\.unwrap\(\)`
Follows up rust-lang#72170 to remove the remaining uses of `lang_items\(\).*\.unwrap\(\)` (avoids a bunch of potential ICEs when working in `#![no_core]`).
Resolvesrust-lang#72195
@cofibrantcofibrant mentioned this pull request May 15, 2020
Open
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 15, 2020
@Dylan-DPC
Rollup merge ofrust-lang#72216- doctorn:require-lang-item, r=lcnr
a671ffe
Remove `lang_items\(\).*\.unwrap\(\)`
Follows up rust-lang#72170 to remove the remaining uses of `lang_items\(\).*\.unwrap\(\)` (avoids a bunch of potential ICEs when working in `#![no_core]`).
Resolvesrust-lang#72195
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 15, 2020
@Dylan-DPC
Rollup merge ofrust-lang#72216- doctorn:require-lang-item, r=lcnr
6105499
Remove `lang_items\(\).*\.unwrap\(\)`
Follows up rust-lang#72170 to remove the remaining uses of `lang_items\(\).*\.unwrap\(\)` (avoids a bunch of potential ICEs when working in `#![no_core]`).
Resolvesrust-lang#72195
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 15, 2020
@Dylan-DPC
Rollup merge ofrust-lang#72216- doctorn:require-lang-item, r=lcnr
ccc62c8
Remove `lang_items\(\).*\.unwrap\(\)`
Follows up rust-lang#72170 to remove the remaining uses of `lang_items\(\).*\.unwrap\(\)` (avoids a bunch of potential ICEs when working in `#![no_core]`).
Resolvesrust-lang#72195
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 16, 2020
@Dylan-DPC
Rollup merge ofrust-lang#72216- doctorn:require-lang-item, r=lcnr
1be88e6
Remove `lang_items\(\).*\.unwrap\(\)`
Follows up rust-lang#72170 to remove the remaining uses of `lang_items\(\).*\.unwrap\(\)` (avoids a bunch of potential ICEs when working in `#![no_core]`).
Resolvesrust-lang#72195
@cofibrantcofibrant mentioned this pull request Jun 4, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RalfJungRalfJungRalfJung approved these changes

Assignees

@RalfJungRalfJung

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Internal compiler error due to failed typecheck (on no_core)

6 participants

@lcnr@RalfJung@oli-obk@bors@jonas-schievink@rust-highfive