Use target_vendor = "apple" instead of target_os = "..."

[madsmtm]

Use target_vendor = "apple" instead of all(target_os = "macos", target_os = "ios", target_os = "tvos", target_os = "watchos", target_os = "visionos").

The apple targets are quite close to being identical, with iOS, tvOS, watchOS and visionOS being even closer, so using target_vendor when possible makes it clearer when something is actually OS-specific, or just Apple-specific.
Note that target_vendor will be deprecated in the future, but not before an alternative (like target_family = "apple") is available.

While doing this, I found various inconsistencies and small mistakes in the standard library, see the commits for details. Will follow-up with an extra PR for a similar issue that need a bit more discussion. EDIT: #124494

Since you've talked about using target_vendor = "apple" in the past:
r? workingjubilee

CC @simlay, @thomcc
@rustbot label O-macos O-ios O-tvos O-watchos O-visionos

[rustbot]

Some changes occurred in src/tools/compiletest

cc @jieyouxu

[QuentinPerez]

Is target_vendor deprecated ? see rust-lang/lang-team#102

[madsmtm]

Is target_vendor deprecated ? see rust-lang/lang-team#102

It isn't yet, I linked the tracking issue in the PR description, it's here, we expect an alternative like target_family = "apple" to be available before it is though, once that does happen it should be easy to find/replace target_vendor = "apple" with that.

[workingjubilee]

...honestly I had a suspicion that this might uncover some bugs but this is impressive.

[workingjubilee]

@madsmtm So, I can r+ this without knowing the answer to this question, but for full transparency: Do the Rust stdlib tests pass on the simulators? Are there any current gaps? It would be good to know what the "current situation" seems to be.

[madsmtm]

Do the Rust stdlib tests pass on the simulators? Are there any current gaps? It would be good to know what the "current situation" seems to be.

It is quite difficult to run a test suite on the simulators, and I'm not experienced enough in Rust's bootstrapping setup to tell you exactly how to do it for this repo (though @simlay probably has some more or less cursed solution using Dinghy), so I'll admit I don't know the answer to your question.

But I'll try to run the test suite under Mac Catalyst, that might reveal some issues.

[workingjubilee]

Ah, I had mostly asked because you had mentioned running some tests in the last commit! I'm happy with that answer, at the moment, and any fixes can be addressed as followups.

@bors r+

[bors]

📌 Commit f9f3573 has been approved by workingjubilee

It is now in the queue for this repository.

[workingjubilee]

Yeah, it seems both are basically just stack pointers, and the rest of the implementation is calls into compiler intrinsics so it is unlikely there is anything that libstd changes can do to make this more or less incorrect.

[thomcc]

I see. Does our va_list type match the declaration in C too? I guess I could check this...

[workingjubilee]

The advantage of merging the libstd changes is that it makes it much easier to test if these implementations are incorrect. :^)

[rust-log-analyzer]

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[bors]

💔 Test failed - checks-actions

[madsmtm]

Modifying Cargo.toml and src/bootstrap/Cargo.toml to depend on a newer version of cc, and running:

./x test --target=aarch64-apple-ios-macabi library/std
./x test --target=aarch64-apple-ios-macabi --no-fail-fast --skip=tests/rustdoc-js --skip=tests/rustdoc-js-std --skip=tests/debuginfo tests

Produces errors in the following tests:

    [codegen] tests/codegen/gdb_debug_script_load.rs
    [run-make] tests/run-make/emit-stack-sizes
    [ui] tests/ui/abi/stack-probes.rs
    [ui] tests/ui/abi/stack-probes-lto.rs
    [ui] tests/ui/command/command-argv0.rs
    [ui] tests/ui/env-funky-keys.rs
    [ui] tests/ui/intrinsics/intrinsic-alignment.rs
    [ui] tests/ui/issues/issue-18804/main.rs
    [ui] tests/ui/link-section.rs
    [ui] tests/ui/linkage-attr/linkage1.rs
    [ui] tests/ui/osx-frameworks.rs
    [ui] tests/ui/runtime/out-of-stack.rs
    [ui] tests/ui/structs-enums/enum-rec/issue-17431-6.rs
    [ui] tests/ui/structs-enums/rec-align-u64.rs

Both before and after this PR. I suspect the stack probing, environment and argv0 to be real issues, and the rest to be under-specified tests that were only tweaked for macOS and not Apple in general. Will likely fix this in another PR at some point.

[madsmtm]

Seems to me like the va_list implementation should use *mut c_void on Apple AArch64 targets, see https://github.com/llvm/llvm-project/blob/llvmorg-18.1.4/clang/lib/Basic/Targets/AArch64.cpp#L1617.

Also, I think CI failed because it was cancelled, or something? Can't really tell from the logs :/

[workingjubilee]

It looks like we do in fact use the right one for the appleOS platforms:

rust/library/core/src/ffi/mod.rs

Lines 207 to 235 in f705de5

	/// Basic implementation of a `va_list`.
	// The name is WIP, using `VaListImpl` for now.
	#[cfg(any(
	all(
	not(target_arch = "aarch64"),
	not(target_arch = "powerpc"),
	not(target_arch = "s390x"),
	not(target_arch = "x86_64")
	),
	all(target_arch = "aarch64", any(target_os = "macos", target_os = "ios", target_os = "tvos")),
	target_family = "wasm",
	target_os = "uefi",
	windows,
	))]
	#[cfg_attr(not(doc), repr(transparent))] // work around https://github.com/rust-lang/rust/issues/90435
	#[unstable(
	feature = "c_variadic",
	reason = "the `c_variadic` feature has not been properly tested on \
	all supported platforms",
	issue = "44930"
	)]
	#[lang = "va_list"]
	pub struct VaListImpl<'f> {
	ptr: *mut c_void,
	// Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
	// the region of the function it's defined in
	_marker: PhantomData<&'f mut &'f c_void>,
	}

Yeah I think it was just a timeout.

@bors retry

[bors]

⌛ Testing commit f9f3573 with merge 2e88e9e...

[bors]

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 2e88e9e to master...

[rust-timer]

Finished benchmarking commit (2e88e9e): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.5%, -0.5%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 672.879s -> 672.43s (-0.07%)
Artifact size: 315.99 MiB -> 315.99 MiB (0.00%)