Implement Clone for CompileOptions (#148)

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`.

Author: simonask

shaderc-rs/src/lib.rs

@@ -76,6 +76,7 @@ use std::any::Any;
 use std::cell::RefCell;
 use std::ffi::{CStr, CString};
 use std::panic;
+use std::rc::Rc;
 use std::{error, fmt, ptr, result, slice, str};
 
 /// Error.
@@ -697,7 +698,7 @@ impl Drop for Compiler {
 pub type IncludeCallbackResult = result::Result<ResolvedInclude, String>;
 
 type BoxedIncludeCallback<'a> =
-    Box<dyn Fn(&str, IncludeType, &str, usize) -> IncludeCallbackResult + 'a>;
+    Rc<dyn Fn(&str, IncludeType, &str, usize) -> IncludeCallbackResult + 'a>;
 
 /// An opaque object managing options to compilation.
 pub struct CompileOptions<'a> {
@@ -753,25 +754,6 @@ impl<'a> CompileOptions<'a> {
         }
     }
 
-    /// Returns a copy of the given compilation options object.
-    ///
-    /// Returns `Err(Error::InitializationError)` if the clone operation failed (underlying call
-    /// returned null).
-    #[allow(clippy::should_implement_trait)]
-    pub fn clone(&self) -> Result<CompileOptions<'a>> {
-        let p = unsafe { scs::shaderc_compile_options_clone(self.raw) };
-        if p.is_null() {
-            Err(Error::InitializationError(
-                "failed to clone CompileOptions".to_string(),
-            ))
-        } else {
-            Ok(CompileOptions {
-                raw: p,
-                include_callback_fn: None,
-            })
-        }
-    }
-
     /// Sets the target environment to `env`, affecting which warnings or errors
     /// will be issued.
     ///
@@ -842,7 +824,7 @@ impl<'a> CompileOptions<'a> {
     {
         use std::mem;
 
-        let f = Box::new(f);
+        let f = Rc::new(f);
         let f_ptr = &*f as *const F;
         self.include_callback_fn = Some(f as BoxedIncludeCallback<'a>);
         unsafe {
@@ -1170,6 +1152,27 @@ impl<'a> CompileOptions<'a> {
     }
 }
 
+impl Clone for CompileOptions<'_> {
+    fn clone(&self) -> Self {
+        let p = unsafe { scs::shaderc_compile_options_clone(self.raw) };
+
+        // This should never happen, outside of a heap allocation failure.
+        // `shaderc_compile_options_clone()` is documented as being identical to
+        // `shaderc_compile_options_init()` when passed a NULL ptr, and there
+        // are no other failure conditions (it is a trivial C++ copy
+        // constructor).
+        assert!(
+            !p.is_null(),
+            "shaderc_compile_options_clone() returned NULL"
+        );
+
+        CompileOptions {
+            raw: p,
+            include_callback_fn: self.include_callback_fn.clone(),
+        }
+    }
+}
+
 impl Drop for CompileOptions<'_> {
     fn drop(&mut self) {
         unsafe { scs::shaderc_compile_options_release(self.raw) }
@@ -1496,7 +1499,7 @@ void main() { my_ssbo.x = 1.0; }";
         let c = Compiler::new().unwrap();
         let mut options = CompileOptions::new().unwrap();
         options.add_macro_definition("E", None);
-        let o = options.clone().unwrap();
+        let o = options.clone();
         let result = c
             .compile_into_spirv_assembly(
                 IFDEF_E,