-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
Make arrays exportable only when their inner type is exportable #875
Conversation
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-875 |
i just realized, in some sense this is technically semver breaking as in code which used to compile now fails. but in another sense, any code that would be broken should hit #870 and thus panic on startup. idk what this counts as |
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.
Thanks!
It looks like doctests are also run for private functions, so export_doctests
should be good! Could you maybe check if the compile_fail
doctest still succeeds if the code actually compiles?
impl<T: ArrayElement> Export for Array<T> | ||
where | ||
T: Export, |
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.
impl<T: ArrayElement> Export for Array<T> | |
where | |
T: Export, | |
impl<T> Export for Array<T> | |
where | |
T: ArrayElement + Export, |
if i comment out the relevant bounds to make both of the exports compile, then the |
Add some compile_fail doctests for `Export`
6bedc41
to
db6bede
Compare
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.
Thanks!
I also added a separate private function under the
Export
trait for some doctests. I wasn't entirely sure it made sense to put them in theExport
trait's docs since they only really exist to test that compilation fails in certain scenarios and it might be a bit weird to just have a bunch of wrong examples.fixes #870