Improve unsize error handling#161
Conversation
| let unsize = self.deps().require_ref::<MirBuiltinEnc>(MirBuiltinEncTask::Unsize(src_ty, *ty, def_id)).unwrap().unsize().unwrap(); | ||
| let unsize = match self.deps().require_ref_spanned::<MirBuiltinEnc>(MirBuiltinEncTask::Unsize(src_ty, *ty, def_id), span) { | ||
| Ok(r) => r.unsize().unwrap(), | ||
| Err(_) => { self.skip_body = true; return; } |
There was a problem hiding this comment.
This seems a bit ad-hoc to do specifically for this one encoder dependency. Maybe deps for the impure encoder should be a wrapper that sets skip_body for any failure?
There was a problem hiding this comment.
I implemented a wrapper, deps_or_skip, with an attribute skip_body which is checked in the visit_* methods.
I had to leave the method deps() as-is due to trait requirements. I replaces deps with deps_or_skip where applicable.
| fn visit_statement(&mut self, statement: &mir::Statement<'vir>, location: mir::Location) { | ||
| self.vcx.with_span(statement.source_info.span, |_vcx| { | ||
| if self.deps.check_cycle().is_err() { | ||
| if self.deps.check_cycle().is_err() || self.skip_body { |
There was a problem hiding this comment.
(If we do this then) should also be done in visit_terminator, and maybe visit_basic_block_data to just avoid the iteration.
|
I guess overall I find it strange that we would add error tracking etc to |
d63ef91 to
4baaee1
Compare
Unsize does always get to an output ref before bailing. My intention was to make this system generic such that it would also work with other encoders that maybe don't manage to produce an output ref. I'm happy to remove it, if you think it's unnecessary |
|
Another point: an alternative to the |
Maybe this is nicer -- the reason we didn't do this is because these methods are methods of the |
|
Done, the code looks much better imo. It should also extend better to other encoders failing, by switching out |
Co-authored-by: Aurea <Aurel300@users.noreply.github.com>
Currently, unsizing is only supported from arrays to slices, e.g.
&[i32; 3] -> &[i32]. Even though #150 tries to improve this situation, it will not make unsizing fully supported. Therefore, it is a good first target to improve user-friendliness of error messages and show partial verification of a file.This PR makes the following changes:
MirBuiltinEncto return a proper error when trying to encode an unsize between unsupported types. This error has sufficient information to implement a properdescribe_errormethodVec<Span>inErrorEnqueueandErrorEncode. Pass aSpantoencode()and record this span for the error case.require_{ref,dep}_spannedis used, the span is passed toencode(). Otherwise, the dummy span is used.program.encoder_errors()to include a span next to the error message.Test Cases
This yields to the following behavior for unsupported unsize operations: