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.
Motivation:
shaderc::CompileOptions::clone()
is actually kind of awkward and not particularly useful, because its return value has the inferred lifetime parameter'_
from the original object, rather than'a
, so the original object must be kept around anyway.As far as I can tell, there is no reason that
shaderc::CompileOptions
could not implement the regularClone
trait, rather than its current specialfn clone() -> Option<Self>
.This is a breaking change, because the signature of the
Clone
trait is different in the following ways:Self
rather thanOption<Self>
. This seems more correct becauseshaderc_compile_options_clone()
cannot fail except for heap allocation failure (it calls a nothrow trivial copy constructor), which is a degenerate scenario regardless.'a
rather than'_
. This seems more correct, since the currentclone()
does not borrow from the existing object.In addition, the inner
include_callback_fn
was changed fromBox<dyn Fn(...)>
to beRc<dyn Fn(...)>
. This should be fine becauseCompileOptions
is already!Send
due to its raw pointer member, and the function isFn
rather thanFnMut
.Despite this being technically a breaking change, I think it might still be worth it. I hope you will agree. :-)