rustc_ast_passes: allow c-variadic associated fns

[dlrobertson]

• Allow c-variadic associated functions
• Add test for c-variadic functions in the Impl context

Related To: #44930
CC: @jethrogb @sarvi
r? @joshtriplett

[joshtriplett]

Please also add some tests that the functions work as expected, retrieving passed variadic arguments.

Ideally, I would love to see the same tests away used for free functions applied to these cases too, perhaps with a macro to avoid duplication.

[dlrobertson]

Please also add some tests that the functions work as expected, retrieving passed variadic arguments.

Ideally, I would love to see the same tests away used for free functions applied to these cases too, perhaps with a macro to avoid duplication.

Sounds good.

[dlrobertson]

@joshtriplett added a run-pass test. Is this what you're looking for?

[bors]

☔ The latest upstream changes (presumably #74862) made this pull request unmergeable. Please resolve the merge conflicts.

[crlf0710]

@dlrobertson Ping from triage, could you address the review comments? thanks

[dlrobertson]

@crlf0710 thanks for the ping, I've also rebased on master.

@joshtriplett I added a run-pass test, but I didn't change the run-make tests. After looking into this a bit more I realized that if the output is a c library we're not going to make symbol foo visible for the function Foo::foo. So I think the only case that this is valid for is a case where the variadics are going to be used entirely within rust.

[Dylan-DPC-zz]

@joshtriplett this is ready for review

[joshtriplett]

@bors r+

[bors]

📌 Commit 96a8048 has been approved by joshtriplett

[jonas-schievink]

@bors r- failed in #77509 (comment)

[dlrobertson]

Looking at this now... not immediately sure why this would fail on arm-android

[Dylan-DPC-zz]

@dlrobertson any updates?

[crlf0710]

@dlrobertson Ping from triage: What's the current status of this?

[dlrobertson]

No updates on why this is failing on android, but after looking at this more I'm not sure why we would add this. A member function wouldn't be exported right? So really this would only be used for a Rust C-variadic function that was used in another rust function. Would that be considered an anti-pattern?

CC @joshtriplett

[tmiasko]

The CStr::from_ptr takes *const c_char as an argument. Whether the char type signed or unsigned is implementation defined in C, and on Android happens to be different from what is being assumed in the test case here.

[joshtriplett]

@dlrobertson wrote:

No updates on why this is failing on android, but after looking at this more I'm not sure why we would add this. A member function wouldn't be exported right? So really this would only be used for a Rust C-variadic function that was used in another rust function. Would that be considered an anti-pattern?

This isn't about member functions; this is about associated functions (which don't take self). There are good reasons to want to have a C-compatible associated function; you might pass such a function as a pointer, for instance.

[JohnCSimon]

Ping from triage
@dlrobertson - hi - is this pull request ready for review? if so can you alert the reviewer, if not, please address the comments. thanks!

[crlf0710]

@dlrobertson Triage: I'm closing this due to inactivity. Feel free to reopen or create a new pr when you've got time to work on this again. Thanks!