Implement Clone for PoolOptions manually (#2548)
#2553
Merged
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
Trying to derive `Clone` automatically for `PoolOptions` results in errors when `clone` is actually called. This is because the derive incorrectly determines that `Clone` is _not_ derivable due to the lack of `Clone` implementation on the `DB: Database` type parameter, even though no value of that type is actually stored in a to-be-cloned position (in fact, it's only used for the `Connection` associated type on the type parameter's `Database` trait impl). Manually implementing `Clone` side-steps this issue and insures the type is always actually cloneable. For reference: #2548
alilleybrinker
changed the title
Clone for PoolOptions manually (#2548)
|
@alilleybrinker can you run |
|
Sure! I've pushed a new commit with |
abonander
requested changes
Jul 6, 2023
sqlx-core/src/pool/options.rs
Outdated
| fn clone(&self) -> Self { | ||
| PoolOptions { | ||
| test_before_acquire: self.test_before_acquire, | ||
| after_connect: self.after_connect.as_ref().map(Arc::clone), |
There was a problem hiding this comment.
Option actually implements Clone which forwards to the inner type: https://doc.rust-lang.org/stable/std/option/enum.Option.html#impl-Clone-for-Option%3CT%3E
So you can change all of these to just .clone()
|
@alilleybrinker did you see my comment? |
|
Yes, I don't think I'll be able to get to this today, but I should be able to do it in the next couple of days. |
|
@abonander Fixed! |
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.
Trying to derive
Cloneautomatically forPoolOptionsresults in errors whencloneis actually called. This is because the derive incorrectly determines thatCloneis not derivable due to the lack ofCloneimplementation on theDB: Databasetype parameter, even though no value of that type is actually stored in a to-be-cloned position (in fact, it's only used for theConnectionassociated type on the type parameter'sDatabasetrait impl).Manually implementing
Cloneside-steps this issue and insures the type is always actually cloneable.For reference: #2548