Skip to content

Fix is_const_context, update check_for_cast #72380

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jun 11, 2020
Merged

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented May 20, 2020

A better version of #71477

Adds fn enclosing_body_owner and uses it in is_const_context.
is_const_context now uses the same mechanism as mir_const_qualif as it was previously incorrect.
Renames is_const_context to is_inside_const_context.

I also updated check_for_cast in the second commit, so r? @estebank

(I removed one lvl of indentation, so it might be easier to review by hiding whitespace changes)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2020
@lcnr lcnr force-pushed the const_context branch from 31513dd to ecb5933 Compare May 20, 2020 14:08
@Elinvynia Elinvynia added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 27, 2020
@Elinvynia Elinvynia added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2020
Comment on lines 40 to 42
LL | let f = [0; -4_isize];
| ^^^^^^^^ expected `usize`, found `isize`
|
help: you can convert an `isize` to `usize` and panic if the converted value wouldn't fit
|
LL | let f = [0; (-4_isize).try_into().unwrap()];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be interesting if we could add a note saying that -4 can't fit in usize, but that's independent of this PR.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 10, 2020

📌 Commit 6da17d2 has been approved by estebank

@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 Jun 10, 2020
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 11, 2020
Fix `is_const_context`, update `check_for_cast`

A better version of rust-lang#71477

Adds `fn enclosing_body_owner` and uses it in `is_const_context`.
`is_const_context` now uses the same mechanism as `mir_const_qualif` as it was previously incorrect.
Renames `is_const_context` to `is_inside_const_context`.

I also updated `check_for_cast` in the second commit, so r? @estebank

(I removed one lvl of indentation, so it might be easier to review by hiding whitespace changes)
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2020
Rollup of 11 pull requests

Successful merges:

 - rust-lang#72380 (Fix `is_const_context`, update `check_for_cast`)
 - rust-lang#72941 (Ensure stack when building MIR for matches)
 - rust-lang#72976 (Clean up E0642 explanation)
 - rust-lang#73080 (doc/rustdoc: Fix incorrect external_doc feature flag)
 - rust-lang#73155 (save_analysis: better handle paths and functions signature)
 - rust-lang#73164 (Add new E0762 error code)
 - rust-lang#73172 (Fix more clippy warnings)
 - rust-lang#73181 (Automatically prioritize unsoundness issues)
 - rust-lang#73183 (Support proc macros in intra doc link resolution)
 - rust-lang#73208 (Fix doctest template)
 - rust-lang#73219 (x.py: with --json-output, forward cargo's JSON)

Failed merges:

r? @ghost
@bors bors merged commit 298467e into rust-lang:master Jun 11, 2020
@lcnr lcnr deleted the const_context branch June 11, 2020 15:03
ayazhafiz added a commit to ayazhafiz/rust that referenced this pull request Jun 13, 2020
re rust-lang#72380 (comment)

Given the toy code

```rust
fn is_positive(n: usize) {
  n > -1_isize;
}
```

We currently get a type mismatch error like the following:

```
error[E0308]: mismatched types
 --> src/main.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^ expected `usize`, found `isize`
  |
help: you can convert an `isize` to `usize` and panic if the converted value wouldn't fit
  |
2 |     n > (-1_isize).try_into().unwrap();
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

But clearly, `-1` can never fit into a `usize`, so the suggestion will
always panic. A more useful message would tell the user that the value
can never fit in the expected type:

```
error[E0308]: mismatched types
 --> test.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^ expected `usize`, found `isize`
  |
note: `-1_isize` can never fit into `usize`
 --> test.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^
```

Which is what this commit implements.

I only added this check for negative literals because

- Currently we can only perform such a check for literals (constant
  value propagation is outside the scope of the typechecker at this
  point)
- A lint error for out-of-range numeric literals is already emitted

IMO it makes more sense to put this check in librustc_lint, but as far
as I can tell the typecheck pass happens before the lint pass, so I've
added it here.

r? @estebank
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 18, 2020
…r=estebank

Note numeric literals that can never fit in an expected type

re rust-lang#72380 (comment)

Given the toy code

```rust
fn is_positive(n: usize) {
  n > -1_isize;
}
```

We currently get a type mismatch error like the following:

```
error[E0308]: mismatched types
 --> src/main.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^ expected `usize`, found `isize`
  |
help: you can convert an `isize` to `usize` and panic if the converted value wouldn't fit
  |
2 |     n > (-1_isize).try_into().unwrap();
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

But clearly, `-1` can never fit into a `usize`, so the suggestion will
always panic. A more useful message would tell the user that the value
can never fit in the expected type:

```
error[E0308]: mismatched types
 --> test.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^ expected `usize`, found `isize`
  |
note: `-1_isize` can never fit into `usize`
 --> test.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^
```

Which is what this commit implements.

I only added this check for negative literals because

- Currently we can only perform such a check for literals (constant
  value propagation is outside the scope of the typechecker at this
  point)
- A lint error for out-of-range numeric literals is already emitted

IMO it makes more sense to put this check in librustc_lint, but as far
as I can tell the typecheck pass happens before the lint pass, so I've
added it here.

r? @estebank
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 19, 2020
…r=estebank

Note numeric literals that can never fit in an expected type

re rust-lang#72380 (comment)

Given the toy code

```rust
fn is_positive(n: usize) {
  n > -1_isize;
}
```

We currently get a type mismatch error like the following:

```
error[E0308]: mismatched types
 --> src/main.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^ expected `usize`, found `isize`
  |
help: you can convert an `isize` to `usize` and panic if the converted value wouldn't fit
  |
2 |     n > (-1_isize).try_into().unwrap();
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

But clearly, `-1` can never fit into a `usize`, so the suggestion will
always panic. A more useful message would tell the user that the value
can never fit in the expected type:

```
error[E0308]: mismatched types
 --> test.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^ expected `usize`, found `isize`
  |
note: `-1_isize` can never fit into `usize`
 --> test.rs:2:9
  |
2 |     n > -1_isize;
  |         ^^^^^^^^
```

Which is what this commit implements.

I only added this check for negative literals because

- Currently we can only perform such a check for literals (constant
  value propagation is outside the scope of the typechecker at this
  point)
- A lint error for out-of-range numeric literals is already emitted

IMO it makes more sense to put this check in librustc_lint, but as far
as I can tell the typecheck pass happens before the lint pass, so I've
added it here.

r? @estebank
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants