clarkmcc / cel-rust

Common Expression Language interpreter written in Rust
https://crates.io/crates/cel-interpreter
MIT License
375 stars 21 forks source link

Fixes #41: Don’t panic on invalid comparisons. #42

Closed chirino closed 4 months ago

chirino commented 5 months ago

This should fix #41

clarkmcc commented 5 months ago

The PartialEq implementation looks good, but I'm thinking that we actually want to return a None on the PartialOrd implementation. I need to check the spec again and see. By using the type ids, we'd be following an ordering scheme that's not strictly defined in the spec, so I'd prefer to return None which triggers a comparison error.

As a note, there are currently a few other places where we have panics, which you can see by searching for unreachable!.

Agreed that nothing should panic.

chirino commented 4 months ago

None it is.