icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Visiting a Slice file with unpatched field crashes #671

Closed externl closed 10 months ago

externl commented 10 months ago

Visiting a file where the AST has a struct with an unpatched field (maybe there are others too) causes the compiler to crash.

Some options:

  1. Fix the code to only partially run the visitor
  2. return an error from visit_with
  3. both 1 & 2
  4. other?

main.rs:

use slicec::{
    compile_from_strings,
    grammar::{
        Class, CustomType, Enum, Enumerator, Exception, Field, Interface, Module, Operation,
        Parameter, Struct, TypeAlias, TypeRef,
    },
    slice_file::SliceFile,
    visitor::Visitor,
};

fn main() {
    let slice = "
    module Igloo
    struct Reading {
        timeStamp: WellKnownTypes::TimeStamp
        temperature: float32
    }";

    let state = compile_from_strings(&[slice], Default::default(), |_| {}, |_| {});
    let file = state.files.get("string-0").expect("file should exist");
    let mut gest = Guest {};
    file.visit_with(&mut gest);
}

struct Guest {}

impl Visitor for Guest {
    fn visit_file(&mut self, _: &SliceFile) {}
    fn visit_module(&mut self, _: &Module) {}
    fn visit_struct(&mut self, _: &Struct) {}
    fn visit_class(&mut self, _: &Class) {}
    fn visit_exception(&mut self, _: &Exception) {}
    fn visit_interface(&mut self, _: &Interface) {}
    fn visit_enum(&mut self, _: &Enum) {}
    fn visit_operation(&mut self, _: &Operation) {}
    fn visit_custom_type(&mut self, _: &CustomType) {}
    fn visit_type_alias(&mut self, _: &TypeAlias) {}
    fn visit_field(&mut self, _: &Field) {}
    fn visit_parameter(&mut self, _: &Parameter) {}
    fn visit_enumerator(&mut self, _: &Enumerator) {}
    fn visit_type_ref(&mut self, _: &TypeRef) {}
}

run main:

thread 'main' panicked at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/grammar/elements/type_ref.rs:20:18:
dereferenced unpatched type reference
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: slicec::grammar::elements::type_ref::TypeRef<T>::definition
             at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/grammar/elements/type_ref.rs:20:18
   3: <slicec::grammar::elements::type_ref::TypeRef<T> as core::ops::deref::Deref>::deref
             at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/grammar/elements/type_ref.rs:75:9
   4: slicec::visitor::<impl slicec::grammar::elements::type_ref::TypeRef>::visit_with
             at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/visitor.rs:271:15
   5: slicec::visitor::<impl slicec::grammar::elements::field::Field>::visit_with
             at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/visitor.rs:241:9
   6: slicec::visitor::<impl slicec::grammar::elements::struct::Struct>::visit_with
             at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/visitor.rs:143:13
   7: slicec::visitor::<impl slicec::slice_file::SliceFile>::visit_with
             at /Users/joe/.cargo/git/checkouts/slicec-c2579922d78ddcad/70b640e/src/visitor.rs:114:51
   8: crash::main
             at ./src/main.rs:22:5
   9: core::ops::function::FnOnce::call_once
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
InsertCreativityHere commented 10 months ago

Yes, I guess it never mattered because in the compilers, we never attempt to run the visitors unless we know the AST is in a completely patched state.

But I agree, it would be nice to be able to at least partially visit things, even if parts of the AST are broken.