move leak-check to during coherence, candidate eval
Implementation of MCP https://github.com/rust-lang/compiler-team/issues/295.
I'd like to do a crater run on this.
Note to @rust-lang/lang: This PR is a breaking change (bugfix). It causes tests like the following to go from a future-compatibility warning #56105 to a hard error:
```rust
trait Trait {}
impl Trait for for<'a, 'b> fn(&'a u32, &'b u32) {}
impl Trait for for<'c> fn(&'c u32, &'c u32) {} // now rejected, used to warn
```
I am not aware of any instances of this code in the wild, but that is why we are doing a crater run. The reason for this change is that those two types are, in fact, the same type, and hence the two impls are overlapping.
There will still be impls that trigger #56105 after this lands, however -- I hope that we will eventually just accept those impls without warning, for the most part. One example of such an impl is this pattern, which is used by wasm-bindgen and other crates as well:
```rust
trait Trait {}
impl<T> Trait for fn(&T) { }
impl<T> Trait for fn(T) { } // still accepted, but warns
```
Improve compiler error message for wrong generic parameter order
- Added optional "help" parameter that shows a help message on the compiler error if required.
- Added a simple ordered parameter as a sample help.
@varkor will make more changes as required. Let me know if I'm heading in the right direction.
Fixes#68437
r? @varkor
Also added FIXME comments to note the possible need to accommodate
counter increment calls in source-based functions that differ from the
function context of the caller instance (e.g., inline functions).
The code should to check that the current expression _is_ the end
expression; not that it's equal to it. The equality check seems very
wasteful in terms of performance.
New lint: suggest `ptr::read` instead of `mem::replace(..., uninitialized())`
resolves: #5575
changelog: improvements to `MEM_REPLACE_WITH_UNINIT`:
- add a new test case in `tests/ui/repl_uninit.rs` to cover the case of replacing with `mem::MaybeUninit::uninit().assume_init()`.
- modify the existing `MEM_REPLACE_WITH_UNINIT` when replacing with `mem::uninitialized` to suggest using `ptr::read` instead.
- lint with `MEM_REPLACE_WITH_UNINIT` when replacing with `mem::MaybeUninit::uninit().assume_init()`
Rollup of 7 pull requests
Successful merges:
- #71756 (add Windows system error codes that should map to io::ErrorKind::TimedOut)
- #73495 (Converted all platform-specific stdin/stdout/stderr implementations to use io:: traits)
- #73575 (Fix typo in error_codes doc)
- #73578 (Make is_freeze and is_copy_modulo_regions take TyCtxtAt)
- #73586 (switch_ty is redundant)
- #73600 (Fix spurious 'value moved here in previous iteration of loop' messages)
- #73610 (Clean up E0699 explanation)
Failed merges:
r? @ghost
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 #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#73553
Fix spurious 'value moved here in previous iteration of loop' messages
Fixes#46099
Previously, we would check the 'move' and 'use' spans to see if we
should emit this message. However, this can give false positives when
macros are involved, since two distinct expressions may end up with the
same span.
Instead, we check the actual MIR `Location`, which eliminates false
positives.
switch_ty is redundant
This field is redundant, but we cannot remove it currently as pretty-printing relies on it (and it does not have access to `mir::Body` to compute the type itself).
Cc @oli-obk @matthewjasper @jonas-schievink
Make is_freeze and is_copy_modulo_regions take TyCtxtAt
Make is_freeze and is_copy_modulo_regions take TyCtxtAt instead of separately taking TyCtxt and Span. This is consistent with is_sized.
Make is_freeze and is_copy_modulo_regions take TyCtxtAt
Make is_freeze and is_copy_modulo_regions take TyCtxtAt instead of separately taking TyCtxt and Span. This is consistent with is_sized.
Converted all platform-specific stdin/stdout/stderr implementations to use io:: traits
Currently, some of the platform-specific standard streams (`src/libstd/sys/*/stdio.rs`) manually implement parts of the `io::Write` interface directly as methods on the struct, rather than by actually implementing the trait. There doesn't seem to be any reason for this, other than an unused advantage of `fn write(&self, ...)` instead of `fn write(&mut self, ...)`.
Unfortunately, this means that those implementations don't have the default-implemented io methods, like `read_exact` and `write_all`. This caused #72705, which adds forwarding methods to the user-facing standard stream implementations, to fail to compile on those platforms.
This change converts *all* such standard stream structs to use the standard library traits. This change should not cause any breakages, because the changed types are not publicly exported, and in fact are only ever used in `src/libstd/io/stdio.rs`.
impl ToSocketAddrs for (String, u16)
This adds a convenience impl of `ToSocketAddrs for (String, u16)`. When authoring HTTP services it's common to take command line options for `host` and `port` and parse them into `String` and `u16` respectively. Consider the following program:
```rust
#[derive(Debug, StructOpt)]
struct Config {
host: String,
port: u16,
}
async fn main() -> io::Result<()> {
let config = Config::from_args();
let stream = TcpStream::connect((&*config.host, config.port))?; // &* is not ideal
// ...
}
```
Networking is a pretty common starting point for people new to Rust, and seeing `&*` in basic examples can be confusing. Even as someone that has experience with networking in Rust I tend to forget that `String` can't be passed directly there. Instead with this patch we can omit the `&*` conversion and pass `host` directly:
```rust
#[derive(Debug, StructOpt)]
struct Config {
host: String,
port: u16,
}
async fn main() -> io::Result<()> {
let config = Config::from_args();
let stream = TcpStream::connect((config.host, config.port))?; // no more conversions!
// ...
}
```
I think should be an easy and small ergonomics improvement for networking. Thanks!
This commit adds a query that allows the CoverageData to be pulled from
a call on tcx, avoiding the need to change the
`codegen_intrinsic_call()` signature (no need to pass in the FunctionCx
or any additional arguments.
The commit does not change where/when the CoverageData is computed. It's
still done in the `pass`, and saved in the MIR `Body`.
See discussion (in progress) here:
https://github.com/rust-lang/rust/pull/73488#discussion_r443825646
The bug was revealed by the behavior of the old-lub-glb-hr-noteq1.rs
test. The old-lub-glb-hr-noteq2 test shows the current 'order dependent'
behavior of coercions around higher-ranked functions, at least when
running with `-Zborrowck=mir`.
Also, run compare-mode=nll.
This improves the output for issue #72577, but there's still more work
to be done.
Currently, an overflow error during monomorphization results in an error
that points at the function we were unable to monomorphize. However, we
don't point at the call that caused the monomorphization to happen. In
the overflow occurs in a large recursive function, it may be difficult
to determine where the issue is.
This commit tracks and `Span` information during collection of
`MonoItem`s, which is used when emitting an overflow error. `MonoItem`
itself is unchanged, so this only affects
`src/librustc_mir/monomorphize/collector.rs`
None of the tools seem to need syn 0.15.35, so we can just build syn
1.0.
This was causing an issue with clippy's `compile-test` program: since
multiple versions of `syn` would exist in the build directory, we would
non-deterministically pick one based on filesystem iteration order. If
the pre-1.0 version of `syn` was picked, a strange build error would
occur (see
https://github.com/rust-lang/rust/pull/73594#issuecomment-647671463)
To prevent this kind of issue from happening again, we now panic if we
find multiple versions of a crate in the build directly, instead of
silently picking the first version we find.
None of the tools seem to need syn 0.15.35, so we can just build syn
1.0.
This was causing an issue with clippy's `compile-test` program: since
multiple versions of `syn` would exist in the build directory, we would
non-deterministically pick one based on filesystem iteration order. If
the pre-1.0 version of `syn` was picked, a strange build error would
occur (see
https://github.com/rust-lang/rust/pull/73594#issuecomment-647671463)
To prevent this kind of issue from happening again, we now panic if we
find multiple versions of a crate in the build directly, instead of
silently picking the first version we find.
Update cargo
3 commits in 79c769c3d7b4c2cf6a93781575b7f592ef974255..089cbb80b73ba242efdcf5430e89f63fa3b5328d
2020-06-11 22:13:37 +0000 to 2020-06-15 14:38:34 +0000
- Support linker with -Zdoctest-xcompile. (rust-lang/cargo#8359)
- Fix doctests not running with --target=HOST. (rust-lang/cargo#8358)
- Allow passing a registry index url directly to `cargo install` (rust-lang/cargo#8344)
Revert the code that states that upcasting traits requires full
equality and change to require that the source type is a subtype of
the target type, as one would expect. As the comment states, this was
an old bug that we didn't want to fix yet as it interacted poorly with
the old leak-check. This fixes the old-lub-glb-object test, which was
previously reporting too many errors (i.e., in the previous commit).
In particular, it no longer occurs during the subtyping check. This is
important for enabling lazy normalization, because the subtyping check
will be producing sub-obligations that could affect its results.
Consider an example like
for<'a> fn(<&'a as Mirror>::Item) =
fn(&'b u8)
where `<T as Mirror>::Item = T` for all `T`. We will wish to produce a
new subobligation like
<'!1 as Mirror>::Item = &'b u8
This will, after being solved, ultimately yield a constraint that `'!1
= 'b` which will fail. But with the leak-check being performed on
subtyping, there is no opportunity to normalize `<'!1 as
Mirror>::Item` (unless we invoke that normalization directly from
within subtyping, and I would prefer that subtyping and unification
are distinct operations rather than part of the trait solving stack).
The reason to keep the leak check during coherence and trait
evaluation is partly for backwards compatibility. The coherence change
permits impls for `fn(T)` and `fn(&T)` to co-exist, and the trait
evaluation change means that we can distinguish those two cases
without ambiguity errors. It also avoids recreating #57639, where we
were incorrectly choosing a where clause that would have failed the
leak check over the impl which succeeds.
The other reason to keep the leak check in those places is that I
think it is actually close to the model we want. To the point, I think
the trait solver ought to have the job of "breaking down"
higher-ranked region obligation like ``!1: '2` into into region
obligations that operate on things in the root universe, at which
point they should be handed off to polonius. The leak check isn't
*really* doing that -- these obligations are still handed to the
region solver to process -- but if/when we do adopt that model, the
decision to pass/fail would be happening in roughly this part of the
code.
This change had somewhat more side-effects than I anticipated. It
seems like there are cases where the leak-check was not being enforced
during method proving and trait selection. I haven't quite tracked
this down but I think it ought to be documented, so that we know what
precisely we are committing to.
One surprising test was `issue-30786.rs`. The behavior there seems a
bit "fishy" to me, but the problem is not related to the leak check
change as far as I can tell, but more to do with the closure signature
inference code and perhaps the associated type projection, which
together seem to be conspiring to produce an unexpected
signature. Nonetheless, it is an example of where changing the
leak-check can have some unexpected consequences: we're now failing to
resolve a method earlier than we were, which suggests we might change
some method resolutions that would have been ambiguous to be
successful.
TODO:
* figure out remainig test failures
* add new coherence tests for the patterns we ARE disallowing
In the new leak check, instead of getting a list of placeholders to
track, we look for any placeholder that is part of a universe which
was created during the snapshot.
We are looking for the following error patterns:
* P1: P2, where P1 != P2
* P1: R, where R is in some universe that cannot name P1
This new leak check is more precise than before, in that it accepts
this patterns:
* R: P1, even if R cannot name P1, because R = 'static is a valid
sol'n
* R: P1, R: P2, as above
Note that this leak check, when running during subtyping, is less
efficient than before in some sense because it is going to check and
re-check all the universes created since the snapshot. We're going to
move when the leak check runs to try and correct that.
