Skip to content

Conversation

cdleary
Copy link
Collaborator

@cdleary cdleary commented Sep 13, 2025

No description provided.

@cdleary cdleary force-pushed the cdleary/2025-09-12-check-test-fn-signature branch from 763bbfd to d789085 Compare September 13, 2025 05:16
@cdleary cdleary force-pushed the cdleary/2025-09-12-check-test-fn-signature branch from d789085 to fea0c17 Compare September 13, 2025 05:22
@cdleary cdleary marked this pull request as ready for review September 13, 2025 05:34
TypeInfo * type_info,
import_data->type_info_owner().GetRootTypeInfo(module.get()));

// Enforce `#[test]` function signature `() -> ()`.
Copy link
Collaborator

@richmckeever richmckeever Sep 13, 2025

Choose a reason for hiding this comment

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

In v2 it would probably make more sense to just add a HandleTestFunction in validate_concrete_type.cc with the logic you have here (starting from where you have the TestFunction ptr). Or perhaps a HandleFunction which does this if the parent is a TestFunction (not sure if the TestFunction itself gets a type).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@richmckeever I tried that at first but it didn't kick in at the right times. I needed to implement it in both places, and then I deleted the Handle* and this one covered the other. I think maybe because it doesn't always need to concretize? It was kind of unclear to me why it wasn't kicking in for all the samples.

@proppy proppy requested a review from richmckeever October 8, 2025 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants