Skip to content

Commit f47b9ab

Browse files
rolandpeelenrolandpeelen
committed
Better error flow
1 parent 755d98d commit f47b9ab

File tree

2 files changed

+108
-105
lines changed

2 files changed

+108
-105
lines changed

‎src/build.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,14 +263,19 @@ fn format_step(current: usize, total: usize) -> console::StyledObject<String> {
263263
#[derive(Debug, Clone)]
264264
pub enum IncrementalBuildError {
265265
SourceFileParseError,
266-
CompileError,
266+
CompileError(Option<String>),
267267
}
268268

269269
impl fmt::Display for IncrementalBuildError {
270270
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
271271
match self {
272272
Self::SourceFileParseError => write!(f, "{} {}Could not parse Source Files", LINE_CLEAR, CROSS,),
273-
Self::CompileError => write!(f, "{} {}Failed to Compile. See Errors Above", LINE_CLEAR, CROSS,),
273+
Self::CompileError(Some(e)) => {
274+
write!(f, "{} {}Failed to Compile. Error: {e}", LINE_CLEAR, CROSS,)
275+
}
276+
Self::CompileError(None) => {
277+
write!(f, "{} {}Failed to Compile. See Errors Above", LINE_CLEAR, CROSS,)
278+
}
274279
}
275280
}
276281
}
@@ -380,7 +385,9 @@ pub fn incremental_build(
380385
};
381386

382387
let (compile_errors, compile_warnings, num_compiled_modules) =
383-
compile::compile(build_state, || pb.inc(1), |size| pb.set_length(size));
388+
compile::compile(build_state, || pb.inc(1), |size| pb.set_length(size))
389+
.map_err(|e| IncrementalBuildError::CompileError(Some(e.to_string())))?;
390+
384391
let compile_duration = start_compiling.elapsed();
385392

386393
logs::finalize(&build_state.packages);
@@ -409,7 +416,7 @@ pub fn incremental_build(
409416
module.compile_dirty = true;
410417
}
411418
}
412-
Err(IncrementalBuildError::CompileError)
419+
Err(IncrementalBuildError::CompileError(None))
413420
} else {
414421
if show_progress {
415422
println!(

‎src/build/compile.rs

Lines changed: 97 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub fn compile(
2020
build_state: &mut BuildState,
2121
inc: impl Fn() + std::marker::Sync,
2222
set_length: impl Fn(u64),
23-
) -> (String, String, usize) {
23+
) -> Result<(String, String, usize)> {
2424
let mut compiled_modules = AHashSet::<String>::new();
2525
let dirty_modules = build_state
2626
.modules
@@ -107,9 +107,9 @@ pub fn compile(
107107

108108
let current_in_progres_modules = in_progress_modules.clone();
109109

110-
current_in_progres_modules
110+
let results = current_in_progres_modules
111111
.par_iter()
112-
.map(|module_name| {
112+
.filter_map(|module_name| {
113113
let module = build_state.get_module(module_name).unwrap();
114114
let package = build_state
115115
.get_package(&module.package_name)
@@ -185,17 +185,8 @@ pub fn compile(
185185
&build_state.project_root,
186186
&build_state.workspace_root,
187187
);
188-
// if let Err(error) = result.to_owned() {
189-
// println!("{}", error);
190-
// panic!("Implementation compilation error!");
191-
// }
192188
let cmi_digest_after = helpers::compute_file_hash(&cmi_path);
193189

194-
// println!(
195-
// "cmi path {}, digest: {:?} / {:?}",
196-
// cmi_path, cmi_digest, cmi_digest_after
197-
// );
198-
199190
// we want to compare both the hash of interface and the implementation
200191
// compile assets to verify that nothing changed. We also need to checke the interface
201192
// because we can include MyModule, so the modules that depend on this module might
@@ -226,105 +217,105 @@ pub fn compile(
226217
}
227218
})
228219
})
229-
.collect::<Vec<_>>()
230-
.iter()
231-
.for_each(|result| {
232-
if let Some((module_name, result, interface_result, is_clean, is_compiled)) = result {
233-
in_progress_modules.remove(module_name);
234-
235-
if *is_compiled {
236-
num_compiled_modules += 1;
237-
}
220+
.collect::<Vec<_>>();
238221

239-
files_current_loop_count += 1;
240-
compiled_modules.insert(module_name.to_string());
222+
for result in results.iter() {
223+
let (module_name, result, interface_result, is_clean, is_compiled) = result;
224+
in_progress_modules.remove(module_name);
241225

242-
if *is_clean {
243-
// actually add it to a list of clean modules
244-
clean_modules.insert(module_name.to_string());
245-
}
226+
if *is_compiled {
227+
num_compiled_modules += 1;
228+
}
229+
230+
files_current_loop_count += 1;
231+
compiled_modules.insert(module_name.to_string());
246232

247-
let module_dependents = build_state.get_module(module_name).unwrap().dependents.clone();
233+
if *is_clean {
234+
// actually add it to a list of clean modules
235+
clean_modules.insert(module_name.to_string());
236+
}
237+
238+
let module_dependents = build_state.get_module(module_name).unwrap().dependents.clone();
248239

249-
// if not clean -- compile modules that depend on this module
250-
for dep in module_dependents.iter() {
251-
// mark the reverse dep as dirty when the source is not clean
252-
if !*is_clean {
253-
let dep_module = build_state.modules.get_mut(dep).unwrap();
254-
// mark the reverse dep as dirty when the source is not clean
255-
dep_module.compile_dirty = true;
240+
// if not clean -- compile modules that depend on this module
241+
for dep in module_dependents.iter() {
242+
// mark the reverse dep as dirty when the source is not clean
243+
if !*is_clean {
244+
let dep_module = build_state.modules.get_mut(dep).unwrap();
245+
// mark the reverse dep as dirty when the source is not clean
246+
dep_module.compile_dirty = true;
247+
}
248+
if !compiled_modules.contains(dep) {
249+
in_progress_modules.insert(dep.to_string());
250+
}
251+
}
252+
253+
let module = build_state
254+
.modules
255+
.get_mut(module_name)
256+
.ok_or(anyhow!("Module not found"))?;
257+
258+
let package = build_state
259+
.packages
260+
.get(&module.package_name)
261+
.ok_or(anyhow!("Package name not found"))?;
262+
263+
match module.source_type {
264+
SourceType::MlMap(ref mut mlmap) => {
265+
module.compile_dirty = false;
266+
mlmap.parse_dirty = false;
267+
}
268+
SourceType::SourceFile(ref mut source_file) => {
269+
match result {
270+
Ok(Some(err)) => {
271+
source_file.implementation.compile_state = CompileState::Warning;
272+
logs::append(package, err);
273+
compile_warnings.push_str(err);
256274
}
257-
if !compiled_modules.contains(dep) {
258-
in_progress_modules.insert(dep.to_string());
275+
Ok(None) => {
276+
source_file.implementation.compile_state = CompileState::Success;
277+
}
278+
Err(err) => {
279+
source_file.implementation.compile_state = CompileState::Error;
280+
logs::append(package, &err.to_string());
281+
compile_errors.push_str(&err.to_string());
282+
}
283+
};
284+
match interface_result {
285+
Some(Ok(Some(err))) => {
286+
source_file.interface.as_mut().unwrap().compile_state = CompileState::Warning;
287+
logs::append(package, &err.to_string());
288+
compile_warnings.push_str(&err.to_string());
289+
}
290+
Some(Ok(None)) => {
291+
if let Some(interface) = source_file.interface.as_mut() {
292+
interface.compile_state = CompileState::Success;
293+
}
259294
}
260-
}
261295

262-
let module = build_state.modules.get_mut(module_name).unwrap();
263-
let package = build_state
264-
.packages
265-
.get(&module.package_name)
266-
.expect("Package not found");
267-
match module.source_type {
268-
SourceType::MlMap(ref mut mlmap) => {
269-
module.compile_dirty = false;
270-
mlmap.parse_dirty = false;
296+
Some(Err(err)) => {
297+
source_file.interface.as_mut().unwrap().compile_state = CompileState::Error;
298+
logs::append(package, &err.to_string());
299+
compile_errors.push_str(&err.to_string());
271300
}
272-
SourceType::SourceFile(ref mut source_file) => {
273-
match result {
274-
Ok(Some(err)) => {
275-
source_file.implementation.compile_state = CompileState::Warning;
276-
logs::append(package, err);
277-
compile_warnings.push_str(err);
278-
}
279-
Ok(None) => {
280-
source_file.implementation.compile_state = CompileState::Success;
281-
}
282-
Err(err) => {
283-
source_file.implementation.compile_state = CompileState::Error;
284-
logs::append(package, &err.to_string());
285-
compile_errors.push_str(&err.to_string());
286-
}
287-
};
288-
match interface_result {
289-
Some(Ok(Some(err))) => {
290-
source_file.interface.as_mut().unwrap().compile_state =
291-
CompileState::Warning;
292-
logs::append(package, &err.to_string());
293-
compile_warnings.push_str(&err.to_string());
294-
}
295-
Some(Ok(None)) => {
296-
if let Some(interface) = source_file.interface.as_mut() {
297-
interface.compile_state = CompileState::Success;
298-
}
299-
}
301+
_ => (),
302+
};
300303

301-
Some(Err(err)) => {
302-
source_file.interface.as_mut().unwrap().compile_state =
303-
CompileState::Error;
304-
logs::append(package, &err.to_string());
305-
compile_errors.push_str(&err.to_string());
306-
}
307-
_ => (),
308-
};
309-
match (result, interface_result) {
310-
// successfull compilation
311-
(Ok(None), Some(Ok(None))) | (Ok(None), None) => {
312-
module.compile_dirty = false;
313-
module.last_compiled_cmi = Some(SystemTime::now());
314-
module.last_compiled_cmt = Some(SystemTime::now());
315-
}
316-
// some error or warning
317-
(Err(_), _)
318-
| (_, Some(Err(_)))
319-
| (Ok(Some(_)), _)
320-
| (_, Some(Ok(Some(_)))) => {
321-
module.compile_dirty = true;
322-
}
323-
}
304+
match (result, interface_result) {
305+
// successfull compilation
306+
(Ok(None), Some(Ok(None))) | (Ok(None), None) => {
307+
module.compile_dirty = false;
308+
module.last_compiled_cmi = Some(SystemTime::now());
309+
module.last_compiled_cmt = Some(SystemTime::now());
310+
}
311+
// some error or warning
312+
(Err(_), _) | (_, Some(Err(_))) | (Ok(Some(_)), _) | (_, Some(Ok(Some(_)))) => {
313+
module.compile_dirty = true;
324314
}
325315
}
326316
}
327-
});
317+
}
318+
}
328319

329320
files_total_count += files_current_loop_count;
330321

@@ -339,6 +330,7 @@ pub fn compile(
339330
.map(|s| (s, build_state.get_module(s).unwrap()))
340331
.collect::<Vec<(&String, &Module)>>(),
341332
);
333+
342334
compile_errors.push_str(&format!(
343335
"\n{}\n{}\n",
344336
style("Can't continue... Found a circular dependency in your code:").red(),
@@ -350,7 +342,7 @@ pub fn compile(
350342
};
351343
}
352344

353-
(compile_errors, compile_warnings, num_compiled_modules)
345+
Ok((compile_errors, compile_warnings, num_compiled_modules))
354346
}
355347

356348
pub fn compiler_args(
@@ -554,7 +546,11 @@ fn compile_file(
554546
let stdout = String::from_utf8_lossy(&x.stdout);
555547
Err(anyhow!(stderr.to_string() + &stdout))
556548
}
557-
Err(e) => Err(anyhow!("Could not compile file. Error: {}. Path to AST: {:?}", e, ast_path)),
549+
Err(e) => Err(anyhow!(
550+
"Could not compile file. Error: {}. Path to AST: {:?}",
551+
e,
552+
ast_path
553+
)),
558554
Ok(x) => {
559555
let err = std::str::from_utf8(&x.stderr)
560556
.expect("stdout should be non-null")

0 commit comments

Comments
 (0)