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 regular Clone trait, rather than its current special fn clone() -> Option<Self>.
This is a breaking change, because the signature of the Clone trait is different in the following ways:
It returns Self rather than Option<Self>. This seems more correct because shaderc_compile_options_clone() cannot fail except for heap allocation failure (it calls a nothrow trivial copy constructor), which is a degenerate scenario regardless.
The lifetime parameter on the return value is 'a rather than '_. This seems more correct, since the current clone() does not borrow from the existing object.
In addition, the inner include_callback_fn was changed from Box<dyn Fn(...)> to be Rc<dyn Fn(...)>. This should be fine because CompileOptions is already !Send due to its raw pointer member, and the function is Fn rather than FnMut.
Despite this being technically a breaking change, I think it might still be worth it. I hope you will agree. :-)
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::CompileOptionscould not implement the regularClonetrait, rather than its current specialfn clone() -> Option<Self>.This is a breaking change, because the signature of the
Clonetrait is different in the following ways:Selfrather 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.'arather than'_. This seems more correct, since the currentclone()does not borrow from the existing object.In addition, the inner
include_callback_fnwas changed fromBox<dyn Fn(...)>to beRc<dyn Fn(...)>. This should be fine becauseCompileOptionsis already!Senddue to its raw pointer member, and the function isFnrather thanFnMut.Despite this being technically a breaking change, I think it might still be worth it. I hope you will agree. :-)