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 6 pull requests #73654

Closed
wants to merge 20 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

GuillaumeGomez and others added 20 commits May 30, 2020 15:36
This is to prevent the miscompilation in rust-lang#73137 from reappearing.
Only runs with `-Zvalidate-mir`.
Co-authored-by: Tyler Mandry <tmandry@gmail.com>
It turns out that this has not been working for who knows how long.
Previously:

```
pub fn h() { 1 + 2; }
```

After this change:

```
pub fn h() { loop {} }
```

This only affected the pass when run with the command line
pretty-printing option, so rustdoc was still replacing bodies with
`loop {}`.
This commit adds a new lint - `improper_ctypes_definitions` - which
functions identically to `improper_ctypes`, but on `extern "C" fn`
definitions (as opposed to `improper_ctypes`'s `extern "C" {}`
declarations).

Signed-off-by: David Wood <david@davidtw.co>
This commit changes the improper ctypes lint (when operating on
definitions) to consider raw pointers or references to sized types as
FFI-safe.

Signed-off-by: David Wood <david@davidtw.co>
This commit adjusts the behaviour introduced in a previous commit so
that generic parameters and projections are only allowed in the
definitions mode - and are otherwise a bug. Generic parameters in
declarations are prohibited earlier in the compiler, so if that branch
were reached, it would be a bug.

Signed-off-by: David Wood <david@davidtw.co>
Currently, rustc uses a heuristic to determine if a range expression is
not a literal based on whether the expression looks like a function call
or struct initialization. This fails for range literals whose
lower/upper bounds are the results of function calls. A possibly-better
heuristic is to check if the expression contains `..`, required in range
literals.

Of course, this is also not perfect; for example, if the range
expression is a struct which includes some text with `..` this will
fail, but in general I believe it is a better heuristic.

A better alternative altogether is to add the `QPath::LangItem` enum
variant suggested in rust-lang#60607. I would be happy to do this as a precursor
to this patch if someone is able to provide general suggestions on how
usages of `QPath` need to be changed later in the compiler with the
`LangItem` variant.

Closes rust-lang#73553
…es-declarations, r=lcnr,varkor

`improper_ctypes_definitions` lint

Addresses rust-lang#19834, rust-lang#66220, and rust-lang#66373.

This PR takes another attempt at rust-lang#65134 (reverted in rust-lang#66378). Instead of modifying the existing `improper_ctypes` lint to consider `extern "C" fn` definitions in addition to `extern "C" {}` declarations, this PR adds a new lint - `improper_ctypes_definitions` - which only applies to `extern "C" fn` definitions.

In addition, the `improper_ctype_definitions` lint differs from `improper_ctypes` by considering `*T` and `&T` (where `T: Sized`) FFI-safe (addressing rust-lang#66220).

There wasn't a clear consensus in rust-lang#66220 (where the issues with rust-lang#65134 were primarily discussed) on the approach to take, but there has [been some discussion in Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.2366220.20improper_ctypes.20definitions.20vs.20declarations/near/198903086). I fully expect that we'll want to iterate on this before landing.

cc @varkor + @shepmaster (from rust-lang#19834) @hanna-kruppe (active in discussing rust-lang#66220), @SimonSapin (rust-lang#65134 caused problems for Servo, want to make sure that this PR doesn't)
…ir, r=tmandry

Check for assignments between non-conflicting generator saved locals

This is to prevent future changes to the generator transform from reintroducing the problem that caused rust-lang#73137. Namely, a store between two generator saved locals whose storage does not conflict.

My ultimate goal is to introduce a modified version of rust-lang#71956 that handles this case properly.

r? @tmandry
…morse

Fix -Z unpretty=everybody_loops

It turns out that this has not been working for who knows how long.
Previously:

```
pub fn h() { 1 + 2; }
```

After this change:

```
pub fn h() { loop { } }
```

This only affected the pass when run with the command line
pretty-printing option, so rustdoc was still replacing bodies with
`loop {}`.
Change heuristic for determining range literal

Currently, rustc uses a heuristic to determine if a range expression is
not a literal based on whether the expression looks like a function call
or struct initialization. This fails for range literals whose
lower/upper bounds are the results of function calls. A possibly-better
heuristic is to check if the expression contains `..`, required in range
literals.

Of course, this is also not perfect; for example, if the range
expression is a struct which includes some text with `..` this will
fail, but in general I believe it is a better heuristic.

A better alternative altogether is to add the `QPath::LangItem` enum
variant suggested in rust-lang#60607. I would be happy to do this as a precursor
to this patch if someone is able to provide general suggestions on how
usages of `QPath` need to be changed later in the compiler with the
`LangItem` variant.

Closes rust-lang#73553
@Dylan-DPC-zz
Copy link
Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented Jun 23, 2020

📌 Commit ec8af88 has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 23, 2020
@bors
Copy link
Contributor

bors commented Jun 23, 2020

⌛ Testing commit ec8af88 with merge d425aa03e0da66c4320429ec7688579e551661a3...

@bors
Copy link
Contributor

bors commented Jun 23, 2020

💔 Test failed - checks-azure

@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 Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

10 participants