Prevent default derives for Forward declared types. #3110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jschwe
force-pushed
the
jschwender/derives_on_fwd_decls
branch
from
c196db6 to
930a6ac
Compare
We do not know the layout for forward declared types. Deriving Copy, Clone, Hash, PartialEq etc. is simply wrong. Deriving Debug is debatable, since it can't print anything useful. Since we previously would derive Debug, lets just keep it, to avoid needless breaking changes. Changed tests: - func-return-must-use: Don't use forward declared structs in the test, since that would result in wrong bindings! - issue-1238-fwd-no-copy test no longer requires an annotation, since Forward declared types should always be no copy - forward-declaration-autoptr.rs: Manually implement clone. The inner type is unknown, so requiring clone/copy is not correct.
jschwe
force-pushed
the
jschwender/derives_on_fwd_decls
branch
from
930a6ac to
102152c
Compare
jschwe
commented
Feb 3, 2025
| @@ -20,7 +20,7 @@ impl<T> Default for RefPtr<T> { | |||
| } | |||
| } | |||
| #[repr(C)] | |||
| #[derive(Debug, Copy, Clone)] | |||
| #[derive(Debug)] | |||
There was a problem hiding this comment.
I think losing Copy, Clone here (on the RefPtr) may be undesired. It seems that bindgen would also need to manually implement the trait instead of deriving it, since Foo is neither Copy nor Clone
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We do not know the layout for forward declared types.
Deriving Copy, Clone, Hash, PartialEq etc. is simply wrong.
Deriving Debug is debatable, since it can't print anything
useful. Since we previously would derive Debug, lets just
keep it, to avoid needless breaking changes.
Changed tests:
test, since that would result in wrong bindings!
since Forward declared types should always be no copy
The inner type is unknown, so requiring clone/copy is not correct.
Fixes #1656