Skip to content

Impl Debug for SendSyncNonNull regardless of pointee - #1498

Open
Jules-Bertholet wants to merge 2 commits into
memorysafety:mainfrom
Jules-Bertholet:sendsyncnonnull-debug
Open

Jules-Bertholet wants to merge 2 commits into
memorysafety:mainfrom
Jules-Bertholet:sendsyncnonnull-debug

Conversation

@Jules-Bertholet

Copy link
Copy Markdown
Contributor

This avoids relying on the Debug impl for c_void, which is dubious; see rust-lang/rust#160115 for details.

@thedataking

Copy link
Copy Markdown
Collaborator

Try rebasing this, CI should be green again now, in part thanks to your fix in #1499.

@thedataking

Copy link
Copy Markdown
Collaborator

This change should wait on your upstream PR rust-lang/rust#160115

@Jules-Bertholet

Copy link
Copy Markdown
Contributor Author

No, it's the opposite; the Rust project doesn't like to break the ecosystem, so that PR (if it goes in at all) needs to wait on this one.

@thedataking

Copy link
Copy Markdown
Collaborator

so that PR (if it goes in at all) needs to wait on this one.

Ack. In that case, let's leave this PR open until you have higher confidence the Rust PR is headed for approval.

I don't love that you're implementing fmt::Debug without calling out why you're not doing the obvious thing via a comment. Did you consider whether we can just get rid of the [derive(Debug)] attribute and leave a comment explaining why? What would be less code for us to understand and maintain.

@Jules-Bertholet

Copy link
Copy Markdown
Contributor Author

Did you consider whether we can just get rid of the [derive(Debug)] attribute

Would mean removing derive(Debug) for c_box::Free also. If you are OK with that, I can do it

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