Ignoring test case: [codegen] repr-transparent-aggregates-1.rs for aarch64

[joaopaulocarreiro]

Ignoring test case: [codegen] repr-transparent-aggregates-1.rs for aarch64.

Copyright (c) 2020, Arm Limited.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[nikomatsakis]

I don't have any objection to ignoring this test -- it looks like a lot of other architectures do so -- but I'm curious what's motivating this PR =)

[hanna-kruppe]

Background for this test file and its twins:

rust/src/test/codegen/repr-transparent.rs

Lines 160 to 169 in 582071c

	// All that remains to be tested are aggregates. They are tested in separate files called repr-
	// transparent-*.rs with `only-*` or `ignore-*` directives, because the expected LLVM IR
	// function signatures vary so much that it's not reasonably possible to cover all of them with a
	// single CHECK line.
	//
	// You may be wondering why we don't just compare the return types and argument types for equality
	// with FileCheck regex captures. Well, rustc doesn't perform newtype unwrapping on newtypes
	// containing aggregates. This is OK on all ABIs we support, but because LLVM has not gotten rid of
	// pointee types yet, the IR function signature will be syntactically different (%Foo* vs
	// %FooWrapper*).

So, it's perfectly fine to ignore this test on AArch64 if the ABI lowering there is different from what's tested in that file, but ideally we'd have a test of this sort for each target (the existing repr-transparent-{1,2,3}.rs tests already cover quite a few targets), so just ignoring is not great IMO.

[joaopaulocarreiro]

The motivation behind the "just ignoring" approach for this PR was based on the fact that I was looking at this as two independent tasks. One of them is to ignore this test for aarch64 (inevitable) and the other is to add suitable tests for the same target. The former is immediate, the latter requires a bit more effort, and is something that should follow.

But if we should proceed in a different way, that's fine too.

[nikomatsakis]

@joaopaulocarreiro can you file a follow-up issue regarding adding the new tests and then assign it to yourself? (via @rustbot claim)

Sorry for being slow to answer, I've fallen behind on notifications and reviews this week.

[nikomatsakis]

@bors delegate

Once that is done, you can feel free to write @bors r=nikomatsakis to merge this PR.

[nikomatsakis]

@bors delegate+

[bors]

✌️ @joaopaulocarreiro can now approve this pull request

[nikomatsakis]

I created #74396 and @joaopaulocarreiro took the liberty of assigning you (you can release assignment if you would like).

@bors r+

[bors]

📌 Commit 582071c has been approved by nikomatsakis

[nikomatsakis]

@bors rollup