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

Check tuple elements are Sized in offset_of #112193

Merged
merged 2 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 10 additions & 9 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3117,16 +3117,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
ty::Tuple(tys) => {
let fstr = field.as_str();

if let Ok(index) = fstr.parse::<usize>() {
if fstr == index.to_string() {
if let Some(&field_ty) = tys.get(index) {
field_indices.push(index.into());
current_container = field_ty;
if let Ok(index) = field.as_str().parse::<usize>()
clubby789 marked this conversation as resolved.
Show resolved Hide resolved
&& field.name == sym::integer(index)
{
for ty in tys.iter().take(index + 1) {
self.require_type_is_sized(ty, expr.span, traits::MiscObligation);
}
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for checking all fields of the tuple (that are before the indexed field)? For ADTs we only check the field we are indexing, and the same should be enough for tuples. So I don't see the point of this loop.

In #126150 I am removing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point I can't remember 🤔 I think all good if it's caught by code elsewhere

if let Some(&field_ty) = tys.get(index) {
field_indices.push(index.into());
current_container = field_ty;

continue;
}
continue;
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/offset-of/offset-of-dst-field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ fn main() {
offset_of!(Alpha, z); //~ ERROR the size for values of type
offset_of!(Beta, z); //~ ERROR the size for values of type
offset_of!(Gamma, z); //~ ERROR the size for values of type
offset_of!((u8, dyn Trait), 0); // ok
offset_of!((u8, dyn Trait), 1); //~ ERROR the size for values of type
}

fn delta() {
Expand Down
19 changes: 14 additions & 5 deletions tests/ui/offset-of/offset-of-dst-field.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,17 @@ LL | offset_of!(Gamma, z);
= help: the trait `Sized` is not implemented for `Extern`
= note: this error originates in the macro `offset_of` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the size for values of type `dyn Trait` cannot be known at compilation time
--> $DIR/offset-of-dst-field.rs:40:5
|
LL | offset_of!((u8, dyn Trait), 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `dyn Trait`
= note: this error originates in the macro `offset_of` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the size for values of type `Extern` cannot be known at compilation time
--> $DIR/offset-of-dst-field.rs:43:5
--> $DIR/offset-of-dst-field.rs:45:5
|
LL | offset_of!(Delta<Extern>, z);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
Expand All @@ -35,7 +44,7 @@ LL | offset_of!(Delta<Extern>, z);
= note: this error originates in the macro `offset_of` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the size for values of type `dyn Trait` cannot be known at compilation time
--> $DIR/offset-of-dst-field.rs:44:5
--> $DIR/offset-of-dst-field.rs:46:5
|
LL | offset_of!(Delta<dyn Trait>, z);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
Expand All @@ -44,7 +53,7 @@ LL | offset_of!(Delta<dyn Trait>, z);
= note: this error originates in the macro `offset_of` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/offset-of-dst-field.rs:42:5
--> $DIR/offset-of-dst-field.rs:44:5
|
LL | offset_of!(Delta<Alpha>, z);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
Expand All @@ -58,7 +67,7 @@ LL | struct Alpha {
= note: this error originates in the macro `offset_of` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the size for values of type `T` cannot be known at compilation time
--> $DIR/offset-of-dst-field.rs:48:5
--> $DIR/offset-of-dst-field.rs:50:5
|
LL | fn generic_with_maybe_sized<T: ?Sized>() -> usize {
| - this type parameter needs to be `std::marker::Sized`
Expand All @@ -72,6 +81,6 @@ LL - fn generic_with_maybe_sized<T: ?Sized>() -> usize {
LL + fn generic_with_maybe_sized<T>() -> usize {
|

error: aborting due to 7 previous errors
error: aborting due to 8 previous errors

For more information about this error, try `rustc --explain E0277`.
10 changes: 10 additions & 0 deletions tests/ui/offset-of/offset-of-tuple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![feature(offset_of)]
#![feature(builtin_syntax)]

fn main() {
core::mem::offset_of!((u8, u8), _0); //~ ERROR no field `_0`
core::mem::offset_of!((u8, u8), +1); //~ ERROR no rules expected
core::mem::offset_of!((u8, u8), -1); //~ ERROR no rules expected
builtin # offset_of((u8, u8), _0); //~ ERROR no field `_0`
builtin # offset_of((u8, u8), +1); //~ ERROR expected identifier
}
37 changes: 37 additions & 0 deletions tests/ui/offset-of/offset-of-tuple.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
error: expected identifier, found `+`
--> $DIR/offset-of-tuple.rs:9:35
|
LL | builtin # offset_of((u8, u8), +1);
| ^ expected identifier

error: no rules expected the token `1`
--> $DIR/offset-of-tuple.rs:6:38
|
LL | core::mem::offset_of!((u8, u8), +1);
| ^ no rules expected this token in macro call
|
= note: while trying to match sequence start

error: no rules expected the token `1`
--> $DIR/offset-of-tuple.rs:7:38
|
LL | core::mem::offset_of!((u8, u8), -1);
| ^ no rules expected this token in macro call
|
= note: while trying to match sequence start

error[E0609]: no field `_0` on type `(u8, u8)`
--> $DIR/offset-of-tuple.rs:5:37
|
LL | core::mem::offset_of!((u8, u8), _0);
| ^^

error[E0609]: no field `_0` on type `(u8, u8)`
--> $DIR/offset-of-tuple.rs:8:35
|
LL | builtin # offset_of((u8, u8), _0);
| ^^

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0609`.
3 changes: 2 additions & 1 deletion tests/ui/offset-of/offset-of-unsized.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// build-pass
// regression test for #112051
// regression test for #112051, not in `offset-of-dst` as the issue is in codegen,
// and isn't triggered in the presence of typeck errors

#![feature(offset_of)]

Expand Down