Skip to content

Move struct param tests to a dedicated file #5539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

bricknerb
Copy link
Contributor

Done in stages for easier review, see commits.

Based on #5537.

Part of #5533.

@bricknerb bricknerb enabled auto-merge May 27, 2025 14:24
@github-actions github-actions bot requested a review from danakj May 27, 2025 14:25
bricknerb added a commit to bricknerb/carbon-lang that referenced this pull request May 27, 2025
@bricknerb bricknerb disabled auto-merge May 27, 2025 14:34

auto foo(S) -> void;

// --- fail_todo_import_struct_decl_param_type.carbon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S is going to need to be complete for a call to foo() to happen, if S is passed by value, so this is never going to pass as written, right?

https://godbolt.org/z/x3e5ao9Y9

auto foo(S*) -> void could pass though. Do you want to write it that way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should never compile.
However, the compilation error should be different, so we still have a TODO.
I made this clearer in #5537.
This PR is just about moving the tests to a different file, so I'll update it after #5537 is merged.


struct F;

F foo();

// --- fail_todo_import_struct_return_type.carbon
// --- fail_todo_import_struct_decl_return_type.carbon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, calling foo() will not ever pass if F is not complete: https://godbolt.org/z/hn5WroTTP

Do you want to change it to auto foo() -> F* or..?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified similarly in #5537.
This PR is just about moving the tests to a different file, so I'll update it after #5537 is merged.

// ============================================================================

// --- struct_return_type.h
// --- struct_decl_return_type.h

struct F;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can be struct S to not confuse things with fn F in the next file split

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #5537.
This PR is just about moving the tests to a different file, so I'll update it after #5537 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants