rustc_layout(debug) does not complain about nonsense types

[RalfJung]

I tried this code:

#![feature(rustc_attrs)]

struct S<T>(T, T);

#[rustc_layout(debug)]
type T = S<str>;

This should complain about using an unsized type in a way that doesn't work. Instead, it prints some layout.

Probably the rustc_layout logic needs to fire of some sort of query to ensure the type is well-formed, or something like that? I know type aliases only get WF-checked "on use", and it seems somehow this attribute doesn't count as a use. How can we make it count as a use?

Cc @oli-obk @lcnr

[fmease]

As a hack, we could treat a type alias with #[rustc_layout] as a lazy type alias ^^ which would fix this

[compiler-errors]

As fmease alluded above, if you're just looking for a convenient way to test the layout of arbitrary types, and don't want to accidentally compute the layout of meaningless types, you should probably just use the #![feature(lazy_type_alias)] gate to make sure that the types are WF-checked at their definition site.

I don't know if it's necessarily worth it to validate these usages in general. It's a rustc attr after all.

[RalfJung]

I was thinking there might be an easy function to call that does whatever happens to a type alias when it is used somewhere that requires WF.

[fmease]

Hmm, I guess we can just call wfcheck::check_item_type if the TyAlias has this attribute. I ruled that out earlier since it's not as “clean” as full lazy type aliases.

[compiler-errors]

I was thinking there might be an easy function to call that does whatever happens to a type alias when it is used somewhere that requires WF.

No, it would at least require spinning up an inference + obligation context and checking a wf goal and reporting a meaningful error if the type being layout'd is not WF. But I don't think it's really even worth that effort.

[RalfJung]

Ah, fair.

The reason I ran into this is that with rustc_abi(assert_eq), this can actually lead to ICEs, as the layout computation code will call delay_span_bug and then later compilation succeeds because the abi-assertion passes. I feel like we should do something about that but I am not sure what.

[oli-obk]

We could emit delay span bugs immediately if rustc_attrs is enabled

[RalfJung]

Then we'll still just get ICEs, that doesn't sound great. These are delayed for a reason, after all.

[oli-obk]

the layout computation code will call delay_span_bug

Hmm... why do we do this on its own instead of also returning an Err from layout computation. There's no real use in producing a broken layout that I can think of, but maybe it helps with something else?

[RalfJung]

I figured out how to do WF-checking, it's not even hard:

pub fn ensure_wf<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool {
    let pred = ty::ClauseKind::WellFormed(ty.into());
    let obligation = traits::Obligation::new(tcx, traits::ObligationCause::dummy(), param_env, pred);
    let infcx = tcx.infer_ctxt().build();
    matches!(infcx.evaluate_obligation_no_overflow(&obligation), traits::EvaluationResult::EvaluatedToOk)
}

However this doesn't print the nice error messages I was hoping for that explain why this isn't WF. Is there an easy way to do that?

[fmease]

You can look at the implementation of wfcheck::check_item_type / wfcheck::enter_wf_checking_ctxt. Basically, infcx.err_ctxt().report_fulfillment_errors and infcx.err_ctxt().resolve_regions_and_report_errors.

[oli-obk]

As a hack, we could treat a type alias with #[rustc_layout] as a lazy type alias ^^ which would fix this

I still think this is the easiest solution

The bug is that we're not wfchecking type aliases, this will check them

[RalfJung]

I'm not sure of the ramifications of this, but subtly changing the semantics of the type alias doesn't seem like a good undocumented side-effect of a debugging macro. That's the kind of thing that introduces the strangest heisenbugs.

[fmease]

The solution with the least amount of code changes would be to call check_item_type in mod wfcheck / TyAlias if tcx.has_attr(def_if, sym::rustc_layout) (which could violate separation of concern?).

Full lazy type aliases would also mean computing the variances, producing a weak projection (AliasTy(Weak)), allowing the lint type_alias_bounds and changing the where-clause location which is not strictly necessary if we're just interested in the body of the type alias. Indeed, I suggested that initially but I've since changed my mind.

This also wouldn't affect downstream users of the #[rustc_layout(debug)] type alias contrary to full-blown lazy ty aliases.

Emitting Wf(ty_alias_body) for those ty aliases is also affecting semantics and therefore also a bit surprising, isn't it?

[RalfJung]

Emitting Wf(ty_alias_body) for those ty aliases is also affecting semantics and therefore also a bit surprising, isn't it?

The only thing it can do is fail compilation because they are not WF, right? So that's fine then.

#115712 fixes the problem for me.

[RalfJung]

Actually this bug was reported before but fixed in an ad-hoc way that didn't resolve the underlying cause: #85103.