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.

Sign up for GitHub

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

Always error on SizeOverflow during mir evaluation #63152

Merged
merged 13 commits into from
Aug 7, 2019
Merged

Always error on SizeOverflow during mir evaluation #63152

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
83b5eb9
Always error on `SizeOverflow` during mir evaluation
estebank Jul 31, 2019
58bd878
Do not lint on SizeOverflow, always error
estebank Jul 31, 2019
d3da411
Nicer labels for type layout errors
estebank Jul 31, 2019
2c56842
avoid mutable state and override main message
estebank Aug 3, 2019
db099fb
Point to local place span on "type too big" error
estebank Aug 3, 2019
2340067
Simplify change to layout_of
estebank Aug 4, 2019
387dcff
review comments: clean up
estebank Aug 4, 2019
bdd79b8
tweak output and tests
estebank Aug 4, 2019
f621f89
revert change to single test
estebank Aug 4, 2019
ce8510a
fix tests
estebank Aug 5, 2019
613c0a8
move error with diverging output to compile-fail
estebank Aug 6, 2019
64469ac
review comments
estebank Aug 6, 2019
3144b0a
review comment: reword test comment
estebank Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/librustc/mir/interpret/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,18 @@ impl<'tcx> ConstEvalErr<'tcx> {
message: &str,
lint_root: Option<hir::HirId>,
) -> Result<DiagnosticBuilder<'tcx>, ErrorHandled> {
let mut must_error = false;
estebank marked this conversation as resolved.
Show resolved Hide resolved
match self.error {
err_inval!(Layout(LayoutError::Unknown(_))) |
err_inval!(TooGeneric) =>
return Err(ErrorHandled::TooGeneric),
err_inval!(Layout(LayoutError::SizeOverflow(_))) |
err_inval!(TypeckError) =>
return Err(ErrorHandled::Reported),
err_inval!(Layout(LayoutError::SizeOverflow(_))) => must_error = true,
estebank marked this conversation as resolved.
Show resolved Hide resolved
_ => {},
}
trace!("reporting const eval failure at {:?}", self.span);
let mut err = if let Some(lint_root) = lint_root {
let mut err = if let (Some(lint_root), false) = (lint_root, must_error) {
let hir_id = self.stacktrace
.iter()
.rev()
Expand Down Expand Up @@ -335,7 +336,7 @@ impl fmt::Debug for InvalidProgramInfo<'tcx> {
TypeckError =>
write!(f, "encountered constants with type errors, stopping evaluation"),
Layout(ref err) =>
write!(f, "rustc layout computation failed: {:?}", err),
write!(f, "{}", err),
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/consts/issue-55878.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// error-pattern: reaching this expression at runtime will panic or abort
fn main() {
println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());
}
14 changes: 14 additions & 0 deletions src/test/ui/consts/issue-55878.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0080]: reaching this expression at runtime will panic or abort
--> $SRC_DIR/libcore/mem/mod.rs:LL:COL
|
LL | intrinsics::size_of::<T>()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ the type `[u8; 18446744073709551615]` is too big for the current architecture
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine merging as is with a FIXME in place for adding a useful (not duplicated) note here. It's likely a bit involeved in the const eval engine, since the way I'm imagining it requires annotating every layout_of call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about that too and came to the same conclusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have a way to make a separate label that won't be too hacky. What do you think a good wording for this error would be/what the end result should look like?

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that there are many situations in const eval that compute layouts and could thus fail. I think we should label each with their own message. If we hardcode a message for just this case it will be nonsensical in all other cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just removed the label altogether and left the main message only

|
::: $DIR/issue-55878.rs:3:26
|
LL | println!("Size: {}", std::mem::size_of::<[u8; std::u64::MAX as usize]>());
| ---------------------------------------------------

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
2 changes: 2 additions & 0 deletions src/test/ui/issues/issue-56762.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ impl TooBigArray {
}

static MY_TOO_BIG_ARRAY_1: TooBigArray = TooBigArray::new();
//~^ ERROR could not evaluate static initializer
static MY_TOO_BIG_ARRAY_2: [u8; HUGE_SIZE] = [0x00; HUGE_SIZE];
//~^ ERROR could not evaluate static initializer

fn main() { }
15 changes: 13 additions & 2 deletions src/test/ui/issues/issue-56762.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
error: the type `[u8; 2305843009213693951]` is too big for the current architecture
error[E0080]: could not evaluate static initializer
--> $DIR/issue-56762.rs:19:1
|
LL | static MY_TOO_BIG_ARRAY_1: TooBigArray = TooBigArray::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the type `[u8; 2305843009213693951]` is too big for the current architecture

error: aborting due to previous error
error[E0080]: could not evaluate static initializer
--> $DIR/issue-56762.rs:21:1
|
LL | static MY_TOO_BIG_ARRAY_2: [u8; HUGE_SIZE] = [0x00; HUGE_SIZE];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the type `[u8; 2305843009213693951]` is too big for the current architecture

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0080`.