-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-107972: Argument Clinic: Ensure a C basename is provided after 'as' #107973
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
gh-107972: Argument Clinic: Ensure a C basename is provided after 'as' #107973
Conversation
erlend-aasland
commented
Aug 15, 2023
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: Argument Clinic silently allow empty custom C basenames #107972
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to Adam's suggestions, but otherwise this LGTM. It appears we do fail()
currently for at least the second case you're adding a test for, but the error message is pretty nonsensical currently:
Couldn't find existing function 'foo2'!
Well, if you'd defined |
Co-authored-by: Adam Turner <[email protected]>
Yup, I gathered. Maybe we should do that in the test, actually, so that the only erroneous thing in the test case is the one we're explicitly testing that clinic catches? |
Try this weird input; clinic accepts it on /*[clinic input]
output everything block
foo2
[clinic start generated code]*/
/*[clinic input]
output everything block
foo as = foo2
[clinic start generated code]*/ |
And with that in mind, we should probably adjust the test case to look exactly like that. |
I did not read this until now; we came to the same conclusion :) |
If it won't backport cleanly, I'm inclined to not backport it. |
Thanks for the reviews! |
Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
Sorry, @erlend-aasland, I could not cleanly backport this to |
Sorry, @erlend-aasland, I could not cleanly backport this to |
|
…er 'as' (python#107973) Co-authored-by: Adam Turner <[email protected]>