inherit #[align] from trait method prototypes

[folkertdev]

@workingjubilee this seems straightforward enough. Now that we're planning to make -Cmin-function-alignment a target modifier, I don't think there are any cross-crate complications here?

@Jules-Bertholet is this the behavior you had in mind? In particular the inheritance of the attribute of a default impl is maybe a bit unintuitive at first? (but I think it's ok if that behavior is explicitly documented).

r? ghost

[Jules-Bertholet]

no_mangle on a trait declaration item is a FCW

[Jules-Bertholet]

(Also, should probably be unsafe(no_mangle))

[folkertdev]

I don't see another way of forcing codegen on the symbol? I'm already using -Clink-dead-code but that is not enough here apparently.

[Jules-Bertholet]

The default method body is implicitly generic in Self, so “forcing codegen on the symbol” doesn’t even really make sense here I think.

[folkertdev]

That's what the

pub struct InstantiateDefaultMethods;

impl T for InstantiateDefaultMethods {
    fn trait_method_inherit_low(self) {}
    fn trait_method_inherit_high(self) {}
}

bit is for.

[workingjubilee]

We should really have a #[rustc_force_codegen] or something.

[workingjubilee]

I'm also not sure how well it would work here, but it would be nicer than #[no_mangle], which is really trying to say an entirely different thing, and -Clink-dead-code.

[folkertdev]

Yes, turns out that does work, but because the name is unmangled, rust inserts an extra comment that also contains the function name, so you have to take that into account when writing the CHECKs.

[Jules-Bertholet]

That's what the […] bit is for.

Imagine if the default body referenced Self, and you had two instantiations for different types. Then we would have two no_mangle symbols with the same name.

[Jules-Bertholet]

Maybe add a few tests for multiple align attributes on the same item, combined with traits? Other than that and my 1 comment, LGTM

[workingjubilee]

@workingjubilee this seems straightforward enough. Now that we're planning to make -Cmin-function-alignment a target modifier, I don't think there are any cross-crate complications here?

@folkertdev I can't promise this won't somehow flow back into concerns for the implementation of the feature we're stabilizing in that PR, but I agree this should be enough for here.

[workingjubilee]

In particular, "there is one universal rule for how this trait's code should be aligned" reduces the number of things we need to worry about when trying to correctly instantiate the alignment of that code's functions.

[folkertdev]

Sure, we'll see how that develops.

At least we've thought about this now, and have something concrete to create tests against if the need arises.

r? @workingjubilee

[rustbot]

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

[rustbot]

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs

cc @jdonszelmann