Skip to content
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

Rollup of 17 pull requests #48350

Closed
wants to merge 46 commits into from
Closed

Conversation

nikomatsakis and others added 30 commits February 12, 2018 14:47
Instead of creating inference variables for those argument types, use
the trait error-reporting code to give a nicer error.
We need two-phase borrows of ops to be in the initial NLL release since without
them lots of existing code will break. Fixes rust-lang#48129
…enums

This dates back to at least rust-lang#26583. At the time, usize and isize were considered ffi-unsafe to nudge people away from them, but this changed in the aforementioned PR, making it inconsistent to complain about it in enum discriminants. In fact, repr(usize) is probably the best way to interface with `enum Foo : size_t { ... }`.
- Always name the non-FFI-safe
- Explain *why* the type is not FFI-safe
- Stop vaguely gesturing at structs/enums/unions if the non-FFI-safe types occured in a field.

The last part is arguably a regression, but it's minor now that the non-FFI-safe type is actually named. Removing it avoids some code duplication.
The suggestion is unconditional, so following it could lead to further errors. This is already the case for the repr(C) suggestion, which makes this acceptable, though not *good*. Checking up-front whether the suggestion can help would be great but applies more broadly (and would require some refactoring to avoid duplicating the checks).
…objects

It's unhelpful since raw pointers to trait objects are also FFI-unsafe and casting to a thin raw pointer loses the vtable. There are working solutions that _involve_ raw pointers but they're too complex to explain in one line and have serious trade offs.
This span covers the whole visibility expression: e.g. `pub (in path)`.
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 20, 2018
@Manishearth
Copy link
Member Author

@bors retry

appveyor reached time limit :|

@bors bors 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 Feb 20, 2018
@bors
Copy link
Contributor

bors commented Feb 20, 2018

⌛ Testing commit bbea863 with merge a9de713be72100a6aa3ca60aef7e88dc14ce23f4...

@bors
Copy link
Contributor

bors commented Feb 20, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: Manishearth
Pushing a9de713be72100a6aa3ca60aef7e88dc14ce23f4 to master...

@Manishearth
Copy link
Member Author

wtf

@Manishearth
Copy link
Member Author

@bors retry

@bors
Copy link
Contributor

bors commented Feb 20, 2018

⌛ Testing commit bbea863 with merge 4ce1b50a783dd3331ddb4d762a5628f90744dbf2...

@bors
Copy link
Contributor

bors commented Feb 20, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: Manishearth
Pushing 4ce1b50a783dd3331ddb4d762a5628f90744dbf2 to master...

@kennytm
Copy link
Member

kennytm commented Feb 20, 2018

@bors retry

@bors
Copy link
Contributor

bors commented Feb 20, 2018

⌛ Testing commit bbea863 with merge 4073992fd970a672c18bf4f8e621be031bdfe4f9...

@bors
Copy link
Contributor

bors commented Feb 20, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 20, 2018
@Manishearth
Copy link
Member Author

@bors retry

time limit

@bors bors 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 Feb 20, 2018
@Manishearth
Copy link
Member Author

(eddyb's MIR PR is not in the rollup so I'll let that land first)

@bors
Copy link
Contributor

bors commented Feb 20, 2018

⌛ Testing commit bbea863 with merge 46c318b...

bors added a commit that referenced this pull request Feb 20, 2018
@bors
Copy link
Contributor

bors commented Feb 20, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 20, 2018
@Manishearth
Copy link
Member Author

Hmm. Perhaps one of these pull requests negatively affects cycle times? Better to not roll up in that case :|

Either that or we're just hitting the limits really often.

@Centril Centril added the rollup A PR which is a rollup label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet