Fix two ICEs by introducing a new layout error variant for unexpected unsized

[FedericoBruzzone]

This PR add the UnexpectedUnsized variant to the LayoutError.
Currently, it allows us to gracefully handle the UnexpectedUnsized error generated by layout_of_struct_or_enum when #![feature(trivial_bounds)] is specified.

With this PR these issues and their respective ICEs are resolved:

• fixes ICE: encountered unexpected unsized field in layout of #135020
• fixes ICE: "Missing value for constant, but no error reported?" with uncomputable layout due to trivial bounds #135138

For instance, given this code:

#![feature(trivial_bounds)]

fn return_str()
where
    str: Sized,
{
    [(); { let _a: Option<str> = None; 0 }];
}

the following error is generated

error[E0080]: evaluation of constant value failed
 --> ice-135138.rs:7:16
  |
7 |     [(); { let _a: Option<str> = None; 0 }];
  |                ^^ cannot determine the layout of the type `Option<str>`

Then, according to @lukas-code: "Keeping this assertion in some sensible manner seems like it would be more effort than it's worth". So, I decided to remove it and the ICE related to #135138 has been fixed.

rust/compiler/rustc_ty_utils/src/layout.rs

Lines 107 to 111 in 6ca6659

	if !field.ty.is_sized(cx.tcx(), cx.typing_env) {
	cx.tcx().dcx().delayed_bug(format!(
	"encountered unexpected unsized field in layout of {ty:?}: {field:#?}"
	));
	}

[rustbot]

r? @cjgillot

rustbot has assigned @cjgillot.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

HIR ty lowering was modified

cc @fmease

[FedericoBruzzone]

Feel free to ask me anything.
In particular, let me know if you would like the addition of a test within the tests/ui/layout/ folder.

[jieyouxu]

(The fluent slug needs to be sorted lexicographically)

[jieyouxu]

error[E0080]: evaluation of constant value failed
 --> ice-135138.rs:7:16
  |
7 |     [(); { let _a: Option<str> = None; 0 }];
  |                ^^ cannot determine the layout of the type `Option<str>`

seems somewhat unfortunate diagnostics-wise because:

• The error is quite generic and isn't super helpful to the user (e.g. why can't the compiler determine the layout?), because it doesn't actually point to the "root" problem, which is Option<str>.
• The problem here is that Option<str> doesn't have a size known at, er, const-eval time?

Introducing this UnexpectedUnsized variant to LayoutError and removing the assertion seems a bit strange to me, won't this paper over genuine implementation bugs, even if this variant is only constructed under trivial_bounds? I do appreciate that the layout calculation is not trivial, so, not a blocker.

Anyway, cc @lukas-code.

[lukas-code]

In particular, let me know if you would like the addition of a test within the tests/ui/layout/ folder.

Yes, this definitely needs tests, for both of the issues. See here for a guide on adding tests.

[lukas-code]

Making a specific variant for UnexpectedUnsized seems to specifically targeted to me. There are many more sources of LayoutError::Unknown and I also managed to trigger the same ICE from this one here:

rust/compiler/rustc_ty_utils/src/layout.rs

Lines 279 to 281 in 243d2ca

	let BackendRepr::Scalar(metadata) = metadata_layout.backend_repr else {
	return Err(error(cx, LayoutError::Unknown(pointee)));
	};

#![feature(ptr_metadata)]
#![feature(trivial_bounds)]

fn return_str()
where
    str: std::ptr::Pointee<Metadata = str>,
{
    [(); { let _a: Option<&str> = None; 0 }];
}

Rather than replacing all the LayoutError::Unknown that can cause this ICE, it might make more sense to add new variant TooGeneric that is only used when attempting to compute the layout of a type parameter or rigid alias (ty::Param / ty::Alias at the bottom of the match in layout_uncached) so that we can be sure that the cause of these is the MIR being too generic and not some weird trait bound or broken type definition and default to reporting all other layout errors except TooGeneric in const eval.

[lukas-code]

Actually, looking at this again, for rigid aliases we can only emit LayoutError::TooGeneric if the type actually contains type parameters (i.e. if ty.has_param()) and need to emit LayoutError::Unknown otherwise, because

for this code, which does not have any type parameters, we should emit the "cannot compute layout of <() as Project>::Assoc" (or whatever it's going to be called) error:

#![feature(trivial_bounds)]

trait Project {
    type Assoc;
}

fn foo() where (): Project {
    // currently ICEs, but should be the new error:
    [(); size_of::<<() as Project>::Assoc>()];
}

and only if there are actual type parameter, then we should emit the "constant expression depends on a generic parameter" error:

trait Project {
    type Assoc;
}

fn foo<T>() where T: Project {
    // current error ("constant expression depends on a generic parameter")
    [(); size_of::<<T as Project>::Assoc>()];
}

[FedericoBruzzone]

Rather than replacing all the LayoutError::Unknown that can cause this ICE, it might make more sense to add new variant TooGeneric that is only used when attempting to compute the layout of a type parameter or rigid alias (ty::Param / ty::Alias at the bottom of the match in layout_uncached) so that we can be sure that the cause of these is the MIR being too generic and not some weird trait bound or broken type definition and default to reporting all other layout errors except TooGeneric in const eval.

That sounds good to me.
As far as ty::Param and ty::Alias is concerned, it is okay.
The ICE in #135020 is generated by layout_of_struct_or_enum which returns LayoutCalculatorError:: UnexpectedUnsized, would you like to handle this as TooGeneric as well?
As far as I know, I don't think that would be correct but let me know.
Maybe we should introduce a new variant to LayoutCalculatorError?

[lukas-code]

by layout_of_struct_or_enum which returns LayoutCalculatorError::UnexpectedUnsized, would you like to handle this as TooGeneric as well?

No, UnexpectedUnsized has nothing to do with generics at all.

My idea behind LayoutError::TooGeneric is that this error means that the layout cannot be computed, because it depends on some generic type parameter T that has not been instantiated yet.

For example

fn foo<T>() {
    [(); size_of::<T>()];
 /*                ^ the layout of the type parameter `T` cannot be computed,
                     therefore the constant `size_of::<T>()` cannot be evaluated */ 
}

And LayoutError::Unknown would be a catch-all, like it currently is. So for example if we attempt to compute the layout of (str, str) that would return LayoutError::Unknown. Or if the layout computation fails for some other reason that is not further specified, that would return LayoutError::Unknown.

The semantic difference between the two would be that LayoutError::TooGeneric means that the layout may become available if the type parameters are further instantiated, i.e. in the example above if we called foo::<i32>() then the layout of the T will become known, but LayoutError::Unknown always means "this type is broken, the layout is meaningless".

Then we can map LayoutError::TooGeneric to ErrorHandled::TooGeneric here:

rust/compiler/rustc_const_eval/src/const_eval/error.rs

Lines 143 to 145 in 3323bbe

	err_inval!(Layout(LayoutError::Unknown(_))) | err_inval!(TooGeneric) => {
	ErrorHandled::TooGeneric(span)
	}

This const eval error basically means "we cannot evaluate this constant because there are uninstantiated generic parameters" and the layout error TooGeneric would mean the same.

On the contrary, LayoutError::Unknown would fall through to the error reporting case below and actually report the error, because no further monomorphization can save us:

rust/compiler/rustc_const_eval/src/const_eval/error.rs

Lines 152 to 176 in 3323bbe

	_ => {
	let (our_span, frames) = get_span_and_frames();
	let span = span.substitute_dummy(our_span);
	let err = mk(span, frames);
	let mut err = tcx.dcx().create_err(err);
	// We allow invalid programs in infallible promoteds since invalid layouts can occur
	// anyway (e.g. due to size overflow). And we allow OOM as that can happen any time.
	let allowed_in_infallible = matches!(
	error,
	InterpErrorKind::ResourceExhaustion(_) | InterpErrorKind::InvalidProgram(_)
	);
	let msg = error.diagnostic_message();
	error.add_args(&mut err);
	// Use *our* span to label the interp error
	err.span_label(our_span, msg);
	let g = err.emit();
	let reported = if allowed_in_infallible {
	ReportedErrorInfo::allowed_in_infallible(g)
	} else {
	ReportedErrorInfo::const_eval_error(g)
	};
	ErrorHandled::Reported(reported, span)
	}

At the same time the new LayoutError::TooGeneric would also be used instead of LayoutError::Unknown in the SizeSkeleton, which is used to typecheck transmute:

rust/compiler/rustc_middle/src/ty/layout.rs

Line 353 in 243d2ca

Err(err @ LayoutError::Unknown(_)) => err,

And for checking the CMSE function signatures for generic parameters:

rust/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs

Line 204 in 243d2ca

Unknown(ty) => {

All so that we can discern layout errors due to "this type has generics" vs "this type in invalid".

---

Maybe we should introduce a new variant to LayoutCalculatorError?

The layout calculator is more low-level than the layout_of_uncached function and does the actual layout computation algorithm. It only knows very basic types (scalars/ADTs) and is unaware of generics (type parameters) for example. The LayoutCalculatorError enum is for errors that stem from the layout optimizing algorithm itself, so adding a new variant there doesn't really seem appropiate.

[FedericoBruzzone]

Okay, so you would like to not handle the issue #135138 in this PR and consequently when the layout_of_struct_or_enum returns LayoutCalculatorError:: UnexpectedUnsized?
So handling just ty::Param & ty::Alias as TooGeneric error (in some way) and the problems that this might raise?

[lukas-code]

Ah, sorry, I understand what you mean now. Yeah my idea is that just ty::Param & ty::Alias (if the alias has a param) are handled as TooGeneric and all other LayoutError::Unknown will stay the same -- including the one from LayoutCalculatorError::UnexpectedUnsized.

Currently, we convert all LayoutError::Unknown (which can happen for various reasons, and not just the type being "too generic") to the const eval error ErrorHandled::TooGeneric and only report the other variants of LayoutError as "failed to evaluate constant" errors. The idea with LayoutError::TooGeneric is that now only layout errors from ty::Param & ty::Alias that are actually "too generic" (in the sense that they contain generic type parameters) are mapped to ErrorHandled::TooGeneric and all remaining LayoutError::Unknowns, including the one from LayoutCalculatorError::UnexpectedUnsized should then be reported as "failed to evaluate constant" errors.

So if you implement LayoutError::TooGeneric like that, then you will fix #135138 without using LayoutError::TooGeneric for LayoutCalculatorError::UnexpectedUnsized (which has nothing to do with generics).

[FedericoBruzzone]

Great! It is exactly as I expected now.
Thank you for the clarification :D