Commit graph

26557 commits

Author SHA1 Message Date
Mazdak Farrokhzad
faf2e8087c
Rollup merge of #64959 - davidtwco:issue-64252-self-type-help, r=Centril,estebank
syntax: improve parameter without type suggestions

Fixes #64252.

This PR improves the suggestions provided when function parameters
do not have types:

- A new suggestion is added for arbitrary self types, which suggests
adding `self: ` before the type.

- Existing suggestions are now provided when a `<` is found where a `:`
was expected (previously only `,` and `)` or trait items), this gives
suggestions in the case where the unnamed parameter type is generic
in a free function.

- The suggestion that a type name be provided (e.g. `fn foo(HashMap<u32>)`
-> `fn foo(HashMap: TypeName<u32>)`) will no longer occur when a `<` was
found instead of `:`.

- The ident will not be used for recovery when a `<` was found instead
of `:`.

r? @Centril
cc @estebank @yoshuawuyts
2019-10-03 04:08:14 +02:00
Mazdak Farrokhzad
17b1fd1a8f
Rollup merge of #64931 - estebank:missing-param-ref, r=matthewjasper,Centril
Reword E0392 slightly

Make it clearer that a type or lifetime argument not being used can be
fixed by referencing it in a struct's fields, not just using `PhathomData`.

CC #53589.
2019-10-03 04:08:12 +02:00
Mazdak Farrokhzad
3e15e51acd
Rollup merge of #63678 - Aaron1011:fix/hrtb-leak, r=nikomatsakis
Improve HRTB error span when -Zno-leak-check is used

As described in #57374, NLL currently produces unhelpful higher-ranked
trait bound (HRTB) errors when '-Zno-leak-check' is enabled.

This PR tackles one half of this issue - making the error message point
at the proper span. The error message itself is still the very generic
"higher-ranked subtype error", but this can be improved in a follow-up
PR.

The root cause of the bad spans lies in how NLL attempts to compute the
'blamed' region, for which it will retrieve a span for.
Consider the following code, which (correctly) does not compile:

```rust
let my_val: u8 = 25;
let a: &u8 = &my_val;
let b = a;
let c = b;
let d: &'static u8 = c;
```

This will cause NLL to generate the following subtype constraints:

d :< c
c :< b
b <: a

Since normal Rust lifetimes are covariant, this results in the following
region constraints (I'm using 'd to denote the lifetime of 'd',
'c to denote the lifetime of 'c, etc.):

'c: 'd
'b: 'c
'a: 'b

From this, we can derive that 'a: 'd holds, which implies that 'a: 'static
must hold. However, this is not the case, since 'a refers to 'my_val',
which does not outlive the current function.

When NLL attempts to infer regions for this code, it will see that the
region 'a has grown 'too large' - it will be inferred to outlive
'static, despite the fact that is not declared as outliving 'static
We can find the region responsible, 'd, by starting at the *end* of
the 'constraint chain' we generated above. This works because for normal
(non-higher-ranked) lifetimes, we generally build up a 'chain' of
lifetime constraints *away* from the original variable/lifetime.
That is, our original lifetime 'a is required to outlive progressively
more regions. If it ends up living for too long, we can look at the
'end' of this chain to determine the 'most recent' usage that caused
the lifetime to grow too large.

However, this logic does not work correctly when higher-ranked trait
bounds (HRTBs) come into play. This is because HRTBs have
*contravariance* with respect to their bound regions. For example,
this code snippet compiles:

```rust
let a: for<'a> fn(&'a ()) = |_| {};
let b: fn(&'static ()) = a;
```

Here, we require that 'a' is a subtype of 'b'. Because of
contravariance, we end up with the region constraint 'static: 'a,
*not* 'a: 'static

This means that our 'constraint chains' grow in the opposite direction
of 'normal lifetime' constraint chains. As we introduce subtypes, our
lifetime ends up being outlived by other lifetimes, rather than
outliving other lifetimes. Therefore, starting at the end of the
'constraint chain' will cause us to 'blame' a lifetime close to the original
definition of a variable, instead of close to where the bad lifetime
constraint is introduced.

This PR improves how we select the region to blame for 'too large'
universal lifetimes, when bound lifetimes are involved. If the region
we're checking is a 'placeholder' region (e.g. the region 'a' in
for<'a>, or the implicit region in fn(&())), we start traversing the
constraint chain from the beginning, rather than the end.

There are two (maybe more) different ways we generate region constraints for NLL:
requirements generated from trait queries, and requirements generated
from MIR subtype constraints. While the former always use explicit
placeholder regions, the latter is more tricky. In order to implement
contravariance for HRTBs, TypeRelating replaces placeholder regions with
existential regions. This requires us to keep track of whether or not an
existential region was originally a placeholder region. When we look for
a region to blame, we check if our starting region is either a
placeholder region or is an existential region created from a
placeholder region. If so, we start iterating from the beginning of the
constraint chain, rather than the end.
2019-10-03 04:08:11 +02:00
Esteban Küber
a180294f8b review comment 2019-10-02 14:34:29 -07:00
Mazdak Farrokhzad
34ea55908e
Rollup merge of #64991 - wesleywiser:fix_too_eager_const_prop, r=oli-obk
[const-prop] Correctly handle locals that can't be propagated

`const_prop()` now handles writing the Rvalue into the Place in the
stack frame for us. So if we're not supposed to propagate that value,
we need to clear it.

r? @oli-obk

Fixes #64970
2019-10-02 18:24:43 +02:00
Mazdak Farrokhzad
028ffd1366
Rollup merge of #64989 - sinkuu:fix_ice_64964, r=davidtwco
Fix ICE #64964

Fixes #64964, which is an ICE with `await`ing in a method + incr-comp.
2019-10-02 18:24:42 +02:00
Mazdak Farrokhzad
7daf2e8946
Rollup merge of #64980 - ecstatic-morse:better-rustc-peek, r=oli-obk
Enable support for `IndirectlyMutableLocals` in `rustc_peek`

This PR allows `rustc_peek` tests to be written for the `IndirectlyMutableLocals` analysis implemented in #64470. See any of the tests in [`test/ui/mir-dataflow`](https://github.com/rust-lang/rust/blob/master/src/test/ui/mir-dataflow/inits-1.rs) for an example.

Included in this PR is a major rewrite of the `rustc_peek` module. This was motivated by the differences between the `IndirectlyMutableLocals` analysis and the initialized places ones.

To properly test `IndirectlyMutableLocals`, we must pass locals by-value to `rustc_peek`, since any local that is not `Freeze` will be marked as indirectly mutable as soon as a reference to it is taken. Unfortunately, `UnsafeCell` is not `Copy`, so we can only do one `rustc_peek` on each value with interior mutability inside a test. I'm not sure how to deal with this restriction; perhaps I need to special case borrows preceding a call to `rustc_peek` in the analysis itself?

`rustc_peek` also assumed that the analysis was done on move paths and that its transfer function only needed to be applied at assignment statements. This PR removes both of those restrictions by adding a trait, `RustcPeekAt`, that controls how the peeked at `Place` maps to the current dataflow state and using a dataflow cursor to retrieve the state itself.

Finally, this PR adds a test which demonstrates some unsoundness in the `IndirectlyMutableLocals` analysis by converting a reference to a `Freeze` field to a reference to a `!Freeze` field by offsetting a pointer (or in this case transmuting a pointer to a ZST field with the same address as a `!Freeze` field). This does not represent a hole in the language proper, since this analysis is only used to validate `const` bodies, in which the unsound code will only compile with `-Zunleash-the-miri-inside-of-you`. Nevertheless, this should get fixed.

r? @oli-obk
2019-10-02 18:24:41 +02:00
Mazdak Farrokhzad
10b0fe9c5c
Rollup merge of #64973 - ecstatic-morse:fix-debuginfo-test, r=alexcrichton
Fix typo while setting `compile-flags` in test

This test is meant to check for an ICE when generating debug info, but didn't actually pass `-g` due to the typo.

I also removed the `FIXME`, since this needs to actually be built (not just checked) to trigger the ICE.
2019-10-02 18:24:39 +02:00
Mazdak Farrokhzad
ccf1d9ca93
Rollup merge of #64967 - ecstatic-morse:issue-64945, r=oli-obk
Don't mark borrows of zero-sized arrays as indirectly mutable

Resolves #64945

r? @oli-obk
2019-10-02 18:24:38 +02:00
Mazdak Farrokhzad
fe5fad8912
Rollup merge of #64914 - pnkfelix:issue-64453-regression-test, r=nikomatsakis
regression test for 64453 borrow check error.

Fix #64453
2019-10-02 18:24:31 +02:00
Mazdak Farrokhzad
18d0c03a13
Rollup merge of #64581 - ztlpn:fix-ok-wrapping-unreachable-code, r=Centril
Fix unreachable_code warnings for try{} block ok-wrapped expressions

Fixes #54165 and fixes #63324.
2019-10-02 18:24:29 +02:00
Wesley Wiser
3a8932d9b0 [const-prop] Correctly handle locals that can't be propagated
`const_prop()` now handles writing the Rvalue into the Place in the
stack frame for us. So if we're not supported to propagate that value,
we need to clear it.
2019-10-02 06:18:45 -04:00
Shotaro Yamada
f0fddb1a89 Do not collect to vec for debug output 2019-10-02 18:59:05 +09:00
Shotaro Yamada
8e67180c52 Fix async/await ICE #64964 2019-10-02 18:26:57 +09:00
David Wood
2537a8aa7a
syntax: improve parameter without type suggestions
This commit improves the suggestions provided when function parameters
do not have types:

- A new suggestion is added for arbitrary self types, which suggests
adding `self: ` before the type.

- Existing suggestions are now provided when a `<` is found where a `:`
was expected (previously only `,` and `)` or trait items), this gives
suggestions in the case where the unnamed parameter type is generic
in a free function.

- The suggestion that a type name be provided (e.g. `fn foo(HashMap<u32>)`
-> `fn foo(HashMap: TypeName<u32>)`) will no longer occur when a `<` was
found instead of `:`.

- The ident will not be used for recovery when a `<` was found instead
of `:`.

Signed-off-by: David Wood <david@davidtw.co>
2019-10-02 09:51:27 +01:00
Tyler Mandry
c11cb25262
Rollup merge of #64649 - estebank:returnator, r=varkor
Avoid ICE on return outside of fn with literal array

Do not ICE when encountering `enum E { A = return [0][0] }`.

Fix #64638.
2019-10-01 23:06:09 -07:00
Dylan MacKenzie
33aa5e855e Add test exposing unsoundness in IndirectlyMutableLocals 2019-10-01 22:16:15 -07:00
Aaron Hill
1caa6dc546
Fixup tests 2019-10-01 21:22:46 -04:00
Aaron Hill
1245467322
Improve HRTB error span when -Zno-leak-check is used
As described in #57374, NLL currently produces unhelpful higher-ranked
trait bound (HRTB) errors when '-Zno-leak-check' is enabled.

This PR tackles one half of this issue - making the error message point
at the proper span. The error message itself is still the very generic
"higher-ranked subtype error", but this can be improved in a follow-up
PR.

The root cause of the bad spans lies in how NLL attempts to compute the
'blamed' region, for which it will retrieve a span for.
Consider the following code, which (correctly) does not compile:

```rust
let my_val: u8 = 25;
let a: &u8 = &my_val;
let b = a;
let c = b;
let d: &'static u8 = c;
```

This will cause NLL to generate the following subtype constraints:

d :< c
c :< b
b <: a

Since normal Rust lifetimes are covariant, this results in the following
region constraints (I'm using 'd to denote the lifetime of 'd',
'c to denote the lifetime of 'c, etc.):

'c: 'd
'b: 'c
'a: 'b

From this, we can derive that 'a: 'd holds, which implies that 'a: 'static
must hold. However, this is not the case, since 'a refers to 'my_val',
which does not outlive the current function.

When NLL attempts to infer regions for this code, it will see that the
region 'a has grown 'too large' - it will be inferred to outlive
'static, despite the fact that is not declared as outliving 'static
We can find the region responsible, 'd, by starting at the *end* of
the 'constraint chain' we generated above. This works because for normal
(non-higher-ranked) lifetimes, we generally build up a 'chain' of
lifetime constraints *away* from the original variable/lifetime.
That is, our original lifetime 'a is required to outlive progressively
more regions. If it ends up living for too long, we can look at the
'end' of this chain to determine the 'most recent' usage that caused
the lifetime to grow too large.

However, this logic does not work correctly when higher-ranked trait
bounds (HRTBs) come into play. This is because HRTBs have
*contravariance* with respect to their bound regions. For example,
this code snippet compiles:

```rust
let a: for<'a> fn(&'a ()) = |_| {};
let b: fn(&'static ()) = a;
```

Here, we require that 'a' is a subtype of 'b'. Because of
contravariance, we end up with the region constraint 'static: 'a,
*not* 'a: 'static

This means that our 'constraint chains' grow in the opposite direction
of 'normal lifetime' constraint chains. As we introduce subtypes, our
lifetime ends up being outlived by other lifetimes, rather than
outliving other lifetimes. Therefore, starting at the end of the
'constraint chain' will cause us to 'blame' a lifetime close to the original
definition of a variable, instead of close to where the bad lifetime
constraint is introduced.

This PR improves how we select the region to blame for 'too large'
universal lifetimes, when bound lifetimes are involved. If the region
we're checking is a 'placeholder' region (e.g. the region 'a' in
for<'a>, or the implicit region in fn(&())), we start traversing the
constraint chain from the beginning, rather than the end.

There are two (maybe more) different ways we generate region constraints for NLL:
requirements generated from trait queries, and requirements generated
from MIR subtype constraints. While the former always use explicit
placeholder regions, the latter is more tricky. In order to implement
contravariance for HRTBs, TypeRelating replaces placeholder regions with
existential regions. This requires us to keep track of whether or not an
existential region was originally a placeholder region. When we look for
a region to blame, we check if our starting region is either a
placeholder region or is an existential region created from a
placeholder region. If so, we start iterating from the beginning of the
constraint chain, rather than the end.
2019-10-01 19:47:28 -04:00
Dylan MacKenzie
cf984d22d6 This needs to be build-pass since it involves debuginfo 2019-10-01 15:10:29 -07:00
Dylan MacKenzie
242702965b Fix typo passing flags to rustc 2019-10-01 15:09:53 -07:00
Dylan MacKenzie
4eeedd0953 Add test cases for #64945
This also tests that `&&[]` no longer causes an ICE in this PR (although
the test fails the borrow checker). This could be more complete.
2019-10-01 14:59:16 -07:00
Mazdak Farrokhzad
67cbeee9ee
Rollup merge of #64933 - sam09:master, r=estebank
Fixes #64919. Suggest fix based on operator precendence.

Fixes https://github.com/rust-lang/rust/issues/64919
2019-10-01 23:56:24 +02:00
Mazdak Farrokhzad
db9689333a
Rollup merge of #64910 - Centril:params-cleanup, r=petrochenkov
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that https://github.com/rust-lang/rust/pull/64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing https://github.com/rust-lang/rust/issues/64252.~ -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
2019-10-01 23:56:21 +02:00
Alex Zatelepin
cb4ed52fd0 fix test after rebase 2019-10-01 17:49:19 +03:00
Alex Zatelepin
ffa526937e address review comments 2019-10-01 17:49:19 +03:00
Alex Zatelepin
c474c6e825 add tests 2019-10-01 17:48:37 +03:00
Felix S. Klock II
9fe138aa43 regression test for 64453 borrow check error. 2019-10-01 15:57:27 +02:00
Sam Radhakrishnan
9e4eb46790 Change to use exprPrecedence instead of exprKind. 2019-10-01 05:00:22 -04:00
Mazdak Farrokhzad
46bf6ad416
Rollup merge of #64937 - estebank:dedup-closure-err, r=Centril
Deduplicate closure type errors

Closure typing obligations flow in both direcitons to properly infer
types. Because of this, we will get 2 type errors whenever there's
an unfulfilled obligation. To avoid this, we deduplicate them in the
`InferCtxt`.
2019-10-01 09:55:39 +02:00
Mazdak Farrokhzad
f4aa29fb1e
Rollup merge of #64930 - davidtwco:issue-61798-diverging-await, r=petrochenkov
Silence unreachable code lint from await desugaring

Fixes #61798.

This PR silences the unreachable code lint when it originates from within an await desugaring.
2019-10-01 09:55:37 +02:00
Mazdak Farrokhzad
dc1c1fe456
Rollup merge of #64928 - JohnTitor:add-some-tests, r=Centril
Add tests for some issues

Closes #50571
Closes #58022
Closes #58344
2019-10-01 09:55:35 +02:00
Mazdak Farrokhzad
24a84fae34
Rollup merge of #64907 - alexreg:tidy-up, r=Mark-Simulacrum
A small amount of tidying-up factored out from PR #64648

As requested by @Mark-Simulacrum, I put this in a separate commit to make it easier to review. (As far as I can tell, no violations of the policy here, and they are simply in a separate PR because they're not directly related to the import of that PR.)

r? @Mark-Simulacrum
2019-10-01 09:55:34 +02:00
Mazdak Farrokhzad
25f774a4cf
Rollup merge of #64895 - davidtwco:issue-64130-async-error-definition, r=nikomatsakis
async/await: improve not-send errors

cc #64130.

```
note: future does not implement `std::marker::Send` because this value is used across an await
  --> $DIR/issue-64130-non-send-future-diags.rs:15:5
   |
LL |     let g = x.lock().unwrap();
   |         - has type `std::sync::MutexGuard<'_, u32>`
LL |     baz().await;
   |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
   | - `g` is later dropped here
```

r? @nikomatsakis
2019-10-01 09:55:31 +02:00
Mazdak Farrokhzad
f45de9a690
Rollup merge of #64887 - Centril:recover-trailing-vert, r=estebank
syntax: recover trailing `|` in or-patterns

Fixes https://github.com/rust-lang/rust/issues/64879.

For example (this also shows that we are sensitive to the typo `||`):

```
error: a trailing `|` is not allowed in an or-pattern
  --> $DIR/remove-leading-vert.rs:33:11
   |
LL |         A || => {}
   |         - ^^ help: remove the `||`
   |         |
   |         while parsing this or-pattern starting here
   |
   = note: alternatives in or-patterns are separated with `|`, not `||`
```

r? @estebank
2019-10-01 09:55:29 +02:00
Mazdak Farrokhzad
3bd34e04e3
Rollup merge of #63931 - petrochenkov:stabmac, r=Centril
Stabilize macros in some more positions

- Fn-like macros and attribute macros in `extern` blocks
- Fn-like procedural macros in type positions
- ~Attribute macros on inline modules~ (moved to https://github.com/rust-lang/rust/pull/64273)

Stabilization report: https://github.com/rust-lang/rust/pull/63931#issuecomment-526362396.

Closes https://github.com/rust-lang/rust/issues/49476
cc https://github.com/rust-lang/rust/issues/54727
2019-10-01 09:55:28 +02:00
Mazdak Farrokhzad
64130fd2c4
Rollup merge of #63674 - petrochenkov:meta2, r=Centril
syntax: Support modern attribute syntax in the `meta` matcher

Where "modern" means https://github.com/rust-lang/rust/pull/57367:
```
PATH
PATH `(` TOKEN_STREAM `)`
PATH `[` TOKEN_STREAM `]`
PATH `{` TOKEN_STREAM `}`
```

Unfortunately, `meta` wasn't future-proofed using the `FOLLOW` token set like other matchers (https://github.com/rust-lang/rust/issues/34011), so code like `$meta:meta {` or `$meta:meta [` may break, and we need a crater run to find out how often this happens in practice.

Closes https://github.com/rust-lang/rust/issues/49629 (by fully supporting `meta` rather than removing it.)
2019-10-01 09:55:26 +02:00
Mazdak Farrokhzad
5c5dd8069d syntax: reformat passing of FnHeader to parse_item_fn. 2019-10-01 06:10:06 +02:00
Esteban Küber
13e9b3deb4 Deduplicate closure type errors
Closure typing obligations flow in both direcitons to properly infer
types. Because of this, we will get 2 type errors whenever there's
an unfulfilled obligation. To avoid this, we deduplicate them in the
`InferCtxt`.
2019-09-30 17:48:22 -07:00
David Wood
04fa9b1b3f
async/await: improve obligation errors
This commit improves obligation errors for async/await:

```
note: future does not implement `std::marker::Send` because this value is used across an
      await
  --> $DIR/issue-64130-non-send-future-diags.rs:15:5
   |
LL |     let g = x.lock().unwrap();
   |         - has type `std::sync::MutexGuard<'_, u32>`
LL |     baz().await;
   |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
   | - `g` is later dropped here
```

Signed-off-by: David Wood <david@davidtw.co>
2019-09-30 23:41:20 +01:00
Sam Radhakrishnan
c9baaa70be Fixes #64919. Suggest fix based on operator precendence. 2019-09-30 18:16:34 -04:00
Esteban Küber
2ae87ff59d Avoid ICE on return outside of fn with literal array
Do not ICE when encountering `enum E { A = return [0][0] }`.
2019-09-30 15:11:19 -07:00
Vadim Petrochenkov
6ea4a52f47 Address review comments 2019-10-01 01:10:12 +03:00
Tyler Mandry
e9d28796ec
Rollup merge of #64921 - JohnTitor:add-test-enum, r=varkor
Add test for issue-64662

Closes #64662
r? @varkor
2019-09-30 14:38:31 -07:00
Tyler Mandry
3add9790ab
Rollup merge of #64838 - GuillaumeGomez:long-err-explanation-e0550, r=oli-obk
Add long error explanation for E0550

Part of #61137
2019-09-30 14:38:24 -07:00
Tyler Mandry
30ba6fd99c
Rollup merge of #64377 - GuillaumeGomez:E0493, r=estebank
Add long error explanation for E0493

Part of #61137.
2019-09-30 14:38:19 -07:00
Esteban Küber
5cbe6aa6ca Reword E0392 slightly
Make it clearer that a type or lifetime argument not being used can be
fixed by referencing it in a struct's fields, not just using `PhathomData`.
2019-09-30 13:35:26 -07:00
David Wood
870b47fee4
typeck: silence unreachable code from await
This commit silences the unreachable code lint when it originates from
within a await desugaring.

Signed-off-by: David Wood <david@davidtw.co>
2019-09-30 21:27:18 +01:00
Vadim Petrochenkov
957986d056 syntax: Support modern attribute syntax in the meta matcher 2019-09-30 22:58:22 +03:00
Yuki Okushi
9c73131fd0 Add test for #58344 2019-10-01 04:55:46 +09:00