no expects or unimpl

examples/compile.rs

@@ -8,6 +8,6 @@ fn main() {
     let mut frag_path = project_root.clone();
     frag_path.push(PathBuf::from("examples/shaders/frag.glsl"));
     let shader = sr::load(vert_path, frag_path).expect("Failed to compile");
-    let vulkano_entry = sr::parse(&shader);
+    let vulkano_entry = sr::parse(&shader).expect("failed to parse");
     dbg!(vulkano_entry);
 }

src/compiler.rs

@@ -2,25 +2,24 @@ use std::fs::File;
 use std::path::Path;
 use shaderc::ShaderKind;
 use std::io::Read;
+use crate::error::CompileError;
 
-pub fn compile<T>(path: T, shader_kind: ShaderKind) -> shaderc::Result<Vec<u32>>
+pub fn compile<T>(path: T, shader_kind: ShaderKind) -> Result<Vec<u32>, CompileError>
 where
     T: AsRef<Path>,
 {
     // TODO Probably shouldn't create this every time.
-    let mut compiler = shaderc::Compiler::new().expect("failed to create compiler");
-    let mut f = File::open(&path).expect("failed to open shader src");
+    let mut compiler = shaderc::Compiler::new().ok_or(CompileError::CreateCompiler)?;
+    let mut f = File::open(&path).map_err(CompileError::Open)?;
     let mut src = String::new();
-    f.read_to_string(&mut src).expect("failed to read src");
-    let mut options = shaderc::CompileOptions::new().unwrap();
-    options.add_macro_definition("EP", Some("main"));
+    f.read_to_string(&mut src).map_err(CompileError::Open)?;
     let result = compiler.compile_into_spirv(
         src.as_str(),
         shader_kind,
-        path.as_ref().to_str().expect("failed to make path string"),
+        path.as_ref().to_str().ok_or(CompileError::InvalidPath)?,
         "main",
         None,
-    )?;
+    ).map_err(CompileError::Compile)?;
     let data = result.as_binary();
     Ok(data.to_owned())
 }

src/error.rs

@@ -1,10 +1,20 @@
 #[derive(Debug)]
 pub enum Error {
-    Compile(shaderc::Error),
+    Compile(CompileError),
     Layout(ConvertError),
+    LoadingData(String),
+    FileWatch(notify::Error),
 }
 
 #[derive(Debug)]
 pub enum ConvertError {
     Unimplemented,
 }
+
+#[derive(Debug)]
+pub enum CompileError {
+    Compile(shaderc::Error),
+    Open(std::io::Error),
+    InvalidPath,
+    CreateCompiler,
+}

src/layouts.rs

@@ -56,14 +56,14 @@ unsafe impl PipelineLayoutDesc for FragLayout {
         self.layout_data.num_sets
     }
     fn num_bindings_in_set(&self, set: usize) -> Option<usize> {
-        self.layout_data.num_bindings.get(&set).map(|&i| i)
+        self.layout_data.num_bindings.get(&set).copied()
     }
     fn descriptor(&self, set: usize, binding: usize) -> Option<DescriptorDesc> {
         self.layout_data.descriptions.get(&set)
             .and_then(|s|s.get(&binding))
             .map(|desc| {
                 let mut desc = desc.clone();
-                desc.stages = self.stages.clone();
+                desc.stages = self.stages;
                 desc
             })
         
@@ -74,8 +74,8 @@ unsafe impl PipelineLayoutDesc for FragLayout {
     fn push_constants_range(&self, num: usize) -> Option<PipelineLayoutDescPcRange> {
         self.layout_data.pc_ranges.get(num)
             .map(|desc| {
-                let mut desc = desc.clone();
-                desc.stages = self.stages.clone();
+                let mut desc = *desc;
+                desc.stages = self.stages;
                 desc
             })
 
@@ -123,14 +123,14 @@ unsafe impl PipelineLayoutDesc for VertLayout {
         self.layout_data.num_sets
     }
     fn num_bindings_in_set(&self, set: usize) -> Option<usize> {
-        self.layout_data.num_bindings.get(&set).map(|&i| i)
+        self.layout_data.num_bindings.get(&set).copied()
     }
     fn descriptor(&self, set: usize, binding: usize) -> Option<DescriptorDesc> {
         self.layout_data.descriptions.get(&set)
             .and_then(|s|s.get(&binding))
             .map(|desc| {
                 let mut desc = desc.clone();
-                desc.stages = self.stages.clone();
+                desc.stages = self.stages;
                 desc
             })
         
@@ -141,8 +141,8 @@ unsafe impl PipelineLayoutDesc for VertLayout {
     fn push_constants_range(&self, num: usize) -> Option<PipelineLayoutDescPcRange> {
         self.layout_data.pc_ranges.get(num)
             .map(|desc| {
-                let mut desc = desc.clone();
-                desc.stages = self.stages.clone();
+                let mut desc = *desc;
+                desc.stages = self.stages;
                 desc
             })
 

src/lib.rs

@@ -24,8 +24,8 @@ pub fn load<T>(vertex: T, fragment: T) -> Result<CompiledShaders, Error>
 where
     T: AsRef<Path>,
 {
-    let vertex = compiler::compile(vertex, ShaderKind::Vertex).map_err(|e| Error::Compile(e))?;
-    let fragment = compiler::compile(fragment, ShaderKind::Fragment).map_err(|e| Error::Compile(e))?;
+    let vertex = compiler::compile(vertex, ShaderKind::Vertex).map_err(Error::Compile)?;
+    let fragment = compiler::compile(fragment, ShaderKind::Fragment).map_err(Error::Compile)?;
     Ok(CompiledShaders{ vertex, fragment })
 }
 

src/reflection.rs

@@ -1,3 +1,4 @@
+use crate::error::Error;
 use crate::layouts::*;
 use crate::sr;
 use crate::srvk::{DescriptorDescInfo, SpirvTy};
@@ -5,7 +6,6 @@ use crate::vk::descriptor::descriptor::*;
 use crate::vk::descriptor::pipeline_layout::PipelineLayoutDescPcRange;
 use crate::vk::pipeline::shader::ShaderInterfaceDefEntry;
 use crate::CompiledShaders;
-use crate::error::Error;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::convert::TryFrom;
@@ -25,9 +25,9 @@ pub struct LayoutData {
 }
 
 pub fn create_entry(shaders: &CompiledShaders) -> Result<Entry, Error> {
-    let vertex_interfaces = create_interfaces(&shaders.vertex);
+    let vertex_interfaces = create_interfaces(&shaders.vertex)?;
     let vertex_layout = create_layouts(&shaders.vertex)?;
-    let fragment_interfaces = create_interfaces(&shaders.fragment);
+    let fragment_interfaces = create_interfaces(&shaders.fragment)?;
     let fragment_layout = create_layouts(&shaders.fragment)?;
     let frag_input = FragInput {
         inputs: fragment_interfaces.inputs,
@@ -65,62 +65,60 @@ pub fn create_entry(shaders: &CompiledShaders) -> Result<Entry, Error> {
     })
 }
 
-fn create_interfaces(data: &[u32]) -> ShaderInterfaces {
+fn create_interfaces(data: &[u32]) -> Result<ShaderInterfaces, Error> {
     sr::ShaderModule::load_u32_data(data)
+        .map_err(|e| Error::LoadingData(e.to_string()))
         .map(|m| {
             let inputs = m
                 .enumerate_input_variables(None)
-                .map(|inputs| {
+                .map_err(|e| Error::LoadingData(e.to_string()))
+                .and_then(|inputs| {
                     inputs
                         .iter()
                         .filter(|i| {
                             !i.decoration_flags
                                 .contains(sr::types::ReflectDecorationFlags::BUILT_IN)
                         })
-                        .map(|i| ShaderInterfaceDefEntry {
+                        .map(|i| Ok(ShaderInterfaceDefEntry {
                             location: i.location..(i.location + 1),
-                            format: SpirvTy::from(i.format).inner(),
+                            format: SpirvTy::try_from(i.format)?.inner(),
                             name: Some(Cow::from(i.name.clone())),
-                        })
-                        .collect::<Vec<ShaderInterfaceDefEntry>>()
-                })
-                .expect("Failed to pass inputs");
+                        }))
+                        .collect::<Result<Vec<ShaderInterfaceDefEntry>, _>>()
+                });
             let outputs = m
                 .enumerate_output_variables(None)
-                .map(|outputs| {
+                .map_err(|e| Error::LoadingData(e.to_string()))
+                .and_then(|outputs| {
                     outputs
                         .iter()
                         .filter(|i| {
                             !i.decoration_flags
                                 .contains(sr::types::ReflectDecorationFlags::BUILT_IN)
                         })
-                        .map(|i| ShaderInterfaceDefEntry {
+                        .map(|i| Ok(ShaderInterfaceDefEntry {
                             location: i.location..(i.location + 1),
-                            format: SpirvTy::from(i.format).inner(),
+                            format: SpirvTy::try_from(i.format)?.inner(),
                             name: Some(Cow::from(i.name.clone())),
-                        })
-                        .collect::<Vec<ShaderInterfaceDefEntry>>()
-                })
-                .expect("Failed to pass outputs");
-            ShaderInterfaces { inputs, outputs }
+                        }))
+                        .collect::<Result<Vec<ShaderInterfaceDefEntry>, _>>()
+                });
+            inputs.and_then(|inputs| outputs.map(|outputs| ShaderInterfaces { inputs, outputs } ))
         })
-        .expect("failed to load module")
+    .and_then(|t| t)
 }
 
 fn create_layouts(data: &[u32]) -> Result<LayoutData, Error> {
     sr::ShaderModule::load_u32_data(data)
         .map(|m| {
-            //let (num_sets, num_bindings, descriptions) = m
-            let descs = 
-                m.enumerate_descriptor_sets(None)
-                .map(|sets| {
+            let descs: Result<_, Error> = m
+                .enumerate_descriptor_sets(None)
+                .map_err(|e| Error::LoadingData(e.to_string()))
+                .and_then(|sets| {
                     let num_sets = sets.len();
                     let num_bindings = sets
                         .iter()
-                        .map(|i| {
-                            dbg!(&i);
-                            (i.set as usize, i.bindings.len())
-                        })
+                        .map(|i| (i.set as usize, i.bindings.len()))
                         .collect::<HashMap<usize, usize>>();
                     let descriptions = sets
                         .iter()
@@ -133,8 +131,7 @@ fn create_layouts(data: &[u32]) -> Result<LayoutData, Error> {
                                         descriptor_type: b.descriptor_type,
                                         image: b.image,
                                     };
-                                    let ty = SpirvTy::<DescriptorDescTy>::try_from(info)?;
-                                    let ty = ty.inner();
+                                    let ty = SpirvTy::<DescriptorDescTy>::try_from(info)?.inner();
                                     let stages = ShaderStages::none();
                                     let d = DescriptorDesc {
                                         ty,
@@ -146,17 +143,15 @@ fn create_layouts(data: &[u32]) -> Result<LayoutData, Error> {
                                     };
                                     Ok((b.binding as usize, d))
                                 })
-                            .flat_map(|d| d.ok())
-                                .collect::<HashMap<usize, DescriptorDesc>>();
-                            (i.set as usize, desc)
+                                .collect::<Result<HashMap<usize, DescriptorDesc>, Error>>();
+                            desc.and_then(|d| Ok((i.set as usize, d)))
                         })
-                        .collect::<HashMap<usize, HashMap<usize, DescriptorDesc>>>();
-                    (num_sets, num_bindings, descriptions)
-                })
-            .into_iter();
-            //let (num_constants, pc_ranges) = m
-            let pcs = 
-                m.enumerate_push_constant_blocks(None)
+                        .collect::<Result<HashMap<usize, _>, Error>>();
+                    descriptions.map(|d| (num_sets, num_bindings, d))
+                });
+            let pcs = m
+                .enumerate_push_constant_blocks(None)
+                .map_err(|e| Error::LoadingData(e.to_string()))
                 .map(|constants| {
                     let num_constants = constants.len();
                     let pc_ranges = constants
@@ -168,15 +163,17 @@ fn create_layouts(data: &[u32]) -> Result<LayoutData, Error> {
                         })
                         .collect::<Vec<PipelineLayoutDescPcRange>>();
                     (num_constants, pc_ranges)
+                });
+            descs.and_then(|(num_sets, num_bindings, descriptions)| {
+                pcs.map(|(num_constants, pc_ranges)| LayoutData {
+                    num_sets,
+                    num_bindings,
+                    descriptions,
+                    num_constants,
+                    pc_ranges,
                 })
-            .into_iter();
-            descs.flat_map(|(num_sets, num_bindings, descriptions)| pcs.map(|(num_constants, pc_ranges)|
-            LayoutData {
-                num_sets,
-                num_bindings,
-                descriptions,
-                num_constants,
-                pc_ranges,
-            })).next()
+            })
         })
+        .map_err(|e| Error::LoadingData(e.to_string()))
+        .and_then(|t| t)
 }

src/srvk.rs

@@ -29,7 +29,7 @@ impl TryFrom<DescriptorDescInfo> for SpirvTy<DescriptorDescTy> {
         match d.descriptor_type {
             SR::Undefined => Err(ConvertError::Unimplemented),
             SR::Sampler => Ok(VK::Sampler),
-            SR::CombinedImageSampler => Ok(VK::CombinedImageSampler(SpirvTy::from(d.image).inner())),
+            SR::CombinedImageSampler => Ok(VK::CombinedImageSampler(SpirvTy::try_from(d.image)?.inner())),
             SR::SampledImage => Err(ConvertError::Unimplemented),
             SR::StorageImage => Err(ConvertError::Unimplemented),
             SR::UniformTexelBuffer => Err(ConvertError::Unimplemented),
@@ -42,12 +42,13 @@ impl TryFrom<DescriptorDescInfo> for SpirvTy<DescriptorDescTy> {
             SR::AccelerationStructureNV => Err(ConvertError::Unimplemented),
         }
         .map(|t| SpirvTy{ inner: t })
-        .map_err(|e| Error::Layout(e))
+        .map_err(Error::Layout)
     }
 }
 
-impl From<sr::types::ReflectImageTraits> for SpirvTy<DescriptorImageDesc> {
-    fn from(d: sr::types::ReflectImageTraits) -> Self {
+impl TryFrom<sr::types::ReflectImageTraits> for SpirvTy<DescriptorImageDesc> {
+    type Error = Error;
+    fn try_from(d: sr::types::ReflectImageTraits) -> Result<Self, Self::Error> {
         let conv_array_layers = |a, d|{
             if a != 0 {
                 DescriptorImageDescArray::Arrayed{max_layers: Some(d)}
@@ -57,29 +58,31 @@ impl From<sr::types::ReflectImageTraits> for SpirvTy<DescriptorImageDesc> {
         };
         let t = DescriptorImageDesc {
             sampled: d.sampled != 0,
-            dimensions: SpirvTy::from(d.dim).inner(),
+            dimensions: SpirvTy::try_from(d.dim)?.inner(),
             // TODO figure out how to do format correctly
             //format: Some(SpirvTy::from(d.image_format).inner()),
             format: None,
             multisampled: d.ms != 0,
             array_layers: conv_array_layers(d.arrayed, d.depth),
         };
-        SpirvTy{inner: t}
+        Ok(SpirvTy{inner: t})
     }
 }
 
-impl From<sr::types::variable::ReflectDimension> for SpirvTy<DescriptorImageDescDimensions> {
-    fn from(d: sr::types::variable::ReflectDimension) -> Self {
+impl TryFrom<sr::types::variable::ReflectDimension> for SpirvTy<DescriptorImageDescDimensions> {
+    type Error = Error;
+    fn try_from(d: sr::types::variable::ReflectDimension) -> Result<Self, Self::Error> {
         use sr::types::variable::ReflectDimension::*;
         use DescriptorImageDescDimensions::*;
-        let inner = match d {
-            Type1d => OneDimensional,
-            Type2d => TwoDimensional,
-            Type3d => ThreeDimensional,
-            sr::types::variable::ReflectDimension::Cube => DescriptorImageDescDimensions::Cube,
-            _ => unimplemented!(),
-        };
-        SpirvTy{ inner }
+        match d {
+            Type1d => Ok(OneDimensional),
+            Type2d => Ok(TwoDimensional),
+            Type3d => Ok(ThreeDimensional),
+            sr::types::variable::ReflectDimension::Cube => Ok(DescriptorImageDescDimensions::Cube),
+            _ => Err(ConvertError::Unimplemented),
+        }
+        .map(|t| SpirvTy{ inner: t })
+        .map_err(Error::Layout)
     }
 }
 
@@ -130,16 +133,20 @@ impl From<sr::types::image::ReflectImageFormat> for SpirvTy<Format> {
             R16_UINT =>R16Uint,
             R8_UINT =>R8Uint,
         };
-        SpirvTy{ inner }
+        SpirvTy{ inner };
+        // This function shouldn't be called yet because 
+        // it is not implemented correctly
+        unreachable!()
     }
 }
 
-impl From<sr::types::ReflectFormat> for SpirvTy<Format> {
-    fn from(f: sr::types::ReflectFormat) -> Self {
+impl TryFrom<sr::types::ReflectFormat> for SpirvTy<Format> {
+    type Error = Error;
+    fn try_from(f: sr::types::ReflectFormat) -> Result<Self, Self::Error> {
         use sr::types::ReflectFormat::*;
         use Format::*;
         let t = match f {
-            Undefined => unimplemented!(),
+            Undefined => Err(Error::Layout(ConvertError::Unimplemented))?,
             R32_UINT => R32Uint,
             R32_SINT => R32Sint,
             R32_SFLOAT => R32Sfloat,
@@ -153,6 +160,6 @@ impl From<sr::types::ReflectFormat> for SpirvTy<Format> {
             R32G32B32A32_SINT => R32G32B32A32Sint,
             R32G32B32A32_SFLOAT => R32G32B32A32Sfloat,
         };
-        SpirvTy { inner: t }
+        Ok(SpirvTy { inner: t })
     }
 }

src/watch.rs

@@ -25,18 +25,18 @@ pub struct Message {
 }
 
 impl Watch {
-    pub fn new<T>(vertex: T, fragment: T) -> Self
+    pub fn create<T>(vertex: T, fragment: T) -> Result<Self, Error>
     where
         T: AsRef<Path>,
     {
         let (handler, rx) = create_watch(
             vertex.as_ref().to_path_buf(),
             fragment.as_ref().to_path_buf(),
-        );
-        Watch {
+        )?;
+        Ok(Watch {
             _handler: handler,
             rx,
-        }
+        })
     }
 }
 
@@ -56,7 +56,8 @@ impl Loader {
         match crate::load(&self.vertex, &self.fragment) {
             Ok(shaders) => {
                 let entry = crate::parse(&shaders);
-                self.tx.send(Ok(Message { shaders, entry })).ok()
+                let msg = entry.map(|entry| Message { shaders, entry });
+                self.tx.send(msg).ok()
             }
             Err(e) => self.tx.send(Err(e)).ok(),
         };
@@ -79,31 +80,33 @@ impl Drop for Handler {
 }
 
 fn create_watch(
-    mut vert_path: PathBuf,
-    mut frag_path: PathBuf,
-) -> (Handler, mpsc::Receiver<Result<Message, Error>>) {
+    vert_path: PathBuf,
+    frag_path: PathBuf,
+) -> Result<(Handler, mpsc::Receiver<Result<Message, Error>>), Error> {
     let (notify_tx, notify_rx) = mpsc::channel();
     let (thread_tx, thread_rx) = mpsc::channel();
     let mut watcher: RecommendedWatcher =
-        Watcher::new(notify_tx, Duration::from_millis(50)).expect("failed to create watcher");
+        Watcher::new(notify_tx, Duration::from_millis(50)).map_err(Error::FileWatch)?;
 
-    vert_path.pop();
-    frag_path.pop();
+    let mut vp = vert_path.clone();
+    let mut fp = frag_path.clone();
+    vp.pop();
+    fp.pop();
     watcher
-        .watch(&vert_path, RecursiveMode::NonRecursive)
-        .expect("failed to add vertex shader to notify");
+        .watch(&vp, RecursiveMode::NonRecursive)
+        .map_err(Error::FileWatch)?;
     watcher
-        .watch(&frag_path, RecursiveMode::NonRecursive)
-        .expect("failed to add fragment shader to notify");
+        .watch(&fp, RecursiveMode::NonRecursive)
+        .map_err(Error::FileWatch)?;
 
     let (loader, rx) = Loader::create(vert_path, frag_path);
 
     let handle = thread::spawn(move || 'watch_loop: loop {
-        if let Ok(_) = thread_rx.try_recv() {
+        if thread_rx.try_recv().is_ok() {
             break 'watch_loop;
         }
-        if let Ok(notify::DebouncedEvent::Create(_))
-        | Ok(notify::DebouncedEvent::Write(_)) = notify_rx.recv_timeout(Duration::from_secs(1))
+        if let Ok(notify::DebouncedEvent::Create(_)) | Ok(notify::DebouncedEvent::Write(_)) =
+            notify_rx.recv_timeout(Duration::from_secs(1))
         {
             loader.reload();
         }
@@ -114,5 +117,5 @@ fn create_watch(
         handle,
         _watcher: watcher,
     };
-    (handler, rx)
+    Ok((handler, rx))
 }

tests/tests.rs

@@ -65,7 +65,7 @@ where
     let mut fragment_path = path.clone();
     fragment_path.push(fragment);
     let shader = shade_runner::load(vertex_path, fragment_path).expect("Failed to compile");
-    shade_runner::parse(&shader)
+    shade_runner::parse(&shader).unwrap()
 }
 
 fn do_test<T>(a: &T, b: &T)