Target modifiers (special marked options) are recorded in metainfo

[azhogin]

Target modifiers (special marked options) are recorded in metainfo and compared to be equal in different linked crates.

PR for this RFC: rust-lang/rfcs#3716

Option may be marked as TARGET_MODIFIER, example: regparm: Option<u32> = (None, parse_opt_number, [TRACKED TARGET_MODIFIER].
If an TARGET_MODIFIER-marked option has non-default value, it will be recorded in crate metainfo as a Vec<TargetModifier>:

pub struct TargetModifier {
    pub opt: OptionsTargetModifiers,
    pub value_name: String,
}

OptionsTargetModifiers is a macro-generated enum.

Option value code (for comparison) is generated using Debug trait.

Error example:

error: mixing `-Zregparm` will cause an ABI mismatch in crate `incompatible_regparm`
  --> $DIR/incompatible_regparm.rs:10:1
   |
LL | #![crate_type = "lib"]
   | ^
   |
   = help: the `-Zregparm` flag modifies the ABI so Rust crates compiled with different values of this flag cannot be used together safely
   = note: `-Zregparm=1` in this crate is incompatible with `-Zregparm=2` in dependency `wrong_regparm`
   = help: set `-Zregparm=2` in this crate or `-Zregparm=1` in `wrong_regparm`
   = help: if you are sure this will not cause problems, use `-Cunsafe-allow-abi-mismatch=regparm` to silence this error

error: aborting due to 1 previous error

-Cunsafe-allow-abi-mismatch=regparm,reg-struct-return to disable list of flags.

[rustbot]

r? @davidtwco

rustbot has assigned @davidtwco.
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

[azhogin]

@rustbot ready

[jieyouxu]

I think the enum key is a lot better, I'll pass this on to someone else just to get another opinion on it.

@davidtwco btw, could you slightly elaborate on what you mean by this? As in, do you want feedback from another reviewer for an aspect of this change in particular, or this PR as a whole (like a second pair of eyes on it)?

[davidtwco]

I think the enum key is a lot better, I'll pass this on to someone else just to get another opinion on it.

@davidtwco btw, could you slightly elaborate on what you mean by this? As in, do you want feedback from another reviewer for an aspect of this change in particular, or this PR as a whole (like a second pair of eyes on it)?

The latter, just to make sure someone else agrees it's going in the right direction

[saethlin]

Suggested change

	/// for ABI-changing or vulnerability-affecting options.
	/// which alter the ABI or effectiveness of exploit mitigations.

I don't think "vulnerability-affecting" is a good term for this. The usual term is "exploit mitigation".

[azhogin]

Fixed.

[saethlin]

I expected this function to just be something like

if !tcx.target_modifiers(LOCAL_CRATE) != tcx.target_modifiers(krate) {
    tcx.emit_err(...
}

but instead, it's way more complicated. Why? What is it doing? It sort of looks like it's trying to come up with a good diagnostic, but I can't even tell if it does the equality check right. So if this is all just in the name of a good diagnostic, can you clearly separate the equality check from the diagnostic reporting? The failure case and this function as a whole is cold, there's no need to avoid looping over flags many times or something like that.

[azhogin]

Yes, it is for good diagnostic.
I have added such a code for clarity:

if mods != *data.target_modifiers() {
    Self::report_target_modifiers_extended(tcx, krate, &mods, data);
}

Also, unsafe_allow_abi_mismatch need to be taken into account to skip allowed mismatches.
-Cunsafe-allow-abi-mismatch=regparm,reg-struct-return.

[azhogin]

@rustbot review

[saethlin]

Thanks! Sorry this PR spent a long time in review. We're still polishing the whole goals process, which should hopefully speed up review for goal-related PRs like this one.

@bors r=davidtwco,saethlin

[bors]

📌 Commit 2a544ab has been approved by davidtwco,saethlin

It is now in the queue for this repository.