Skip to content

Commit 2fdd916

Browse files
committed
Bug fixes:
- compiler: fix if statement miscompilation - deserialization: replace panic with error when serialized entrries dont have ids - point dragging: dont reset dragging state to drag x when both drag dirs become unavailable
1 parent 1f77261 commit 2fdd916

6 files changed

Lines changed: 55 additions & 44 deletions

File tree

evalexpr/src/flat_node/variable_inlining.rs

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -593,21 +593,21 @@ pub fn inline_variables_and_fold<F: EvalexprFloat>(
593593
FlatOperator::Leq => {
594594
fold_binary_op(&mut new_ops, source_op, eval::less_eq)?;
595595
},
596-
FlatOperator::Eq => {
597-
fold_binary_op(&mut new_ops, source_op,|a,b| Ok(eval::eq(a,b)))?;
598-
},
599-
FlatOperator::Neq => {
600-
fold_binary_op(&mut new_ops, source_op,|a,b| Ok(eval::neq(a,b)))?;
601-
},
602-
FlatOperator::And => {
603-
fold_binary_op(&mut new_ops, source_op, eval::and)?;
604-
},
605-
FlatOperator::Or => {
606-
fold_binary_op(&mut new_ops, source_op, eval::or)?;
607-
},
608-
FlatOperator::Not => {
609-
fold_unary_op(&mut new_ops, source_op, eval::not)?;
610-
},
596+
FlatOperator::Eq => {
597+
fold_binary_op(&mut new_ops, source_op, |a, b| Ok(eval::eq(a, b)))?;
598+
},
599+
FlatOperator::Neq => {
600+
fold_binary_op(&mut new_ops, source_op, |a, b| Ok(eval::neq(a, b)))?;
601+
},
602+
FlatOperator::And => {
603+
fold_binary_op(&mut new_ops, source_op, eval::and)?;
604+
},
605+
FlatOperator::Or => {
606+
fold_binary_op(&mut new_ops, source_op, eval::or)?;
607+
},
608+
FlatOperator::Not => {
609+
fold_unary_op(&mut new_ops, source_op, eval::not)?;
610+
},
611611

612612
FlatOperator::Tuple { len } => {
613613
if *len == 2 {
@@ -831,10 +831,14 @@ fn fold_binary_op_with_two_const_variants<F: EvalexprFloat>(
831831
new_ops.pop().unwrap();
832832
new_ops.push(fold_1_1(last_const));
833833
} else if let Some((second_last_const, idx)) = get_second_last_if_const(new_ops) {
834-
// println!("State before {new_ops:?}");
835-
// println!("folding secondd last const {op:?} removing {idx}");
836-
new_ops.remove(idx);
837-
new_ops.push(fold_1_2(second_last_const));
834+
if is_last_op_label(new_ops) {
835+
new_ops.push(op.clone());
836+
} else {
837+
// println!("State before {new_ops:?}");
838+
// println!("folding secondd last const {op:?} removing {idx}");
839+
new_ops.remove(idx);
840+
new_ops.push(fold_1_2(second_last_const));
841+
}
838842
// println!("State after {new_ops:?}");
839843
} else {
840844
new_ops.push(op.clone());
@@ -892,6 +896,12 @@ fn fold_ternary_op_with_partial<F: EvalexprFloat>(
892896
Ok(())
893897
}
894898

899+
fn is_last_op_label<F: EvalexprFloat>(ops: &[FlatOperator<F>]) -> bool {
900+
if let Some(FlatOperator::Label { .. }) = ops.last() {
901+
return true;
902+
}
903+
false
904+
}
895905
fn get_last_if_const<F: EvalexprFloat>(ops: &[FlatOperator<F>]) -> Option<Value<F>> {
896906
let Some(last) = ops.last() else {
897907
panic!("Must have last op");

src/app_ui.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,11 @@ pub fn side_panel<T: EvalexprFloat>(
305305
});
306306

307307
ui.separator();
308+
if let Some(error) = &ui_state.serialization_error {
309+
if ui.label(RichText::new(error).color(Color32::RED)).clicked() {
310+
ui_state.serialization_error = None;
311+
}
312+
}
308313
#[cfg(target_arch = "wasm32")]
309314
const PERSISTANCE_TYPE: &str = "Local Storage";
310315
#[cfg(not(target_arch = "wasm32"))]
@@ -313,11 +318,6 @@ pub fn side_panel<T: EvalexprFloat>(
313318
persistence::persistence_ui(state, ui_state, ui, frame);
314319
});
315320

316-
if let Some(error) = &ui_state.serialization_error {
317-
if ui.label(RichText::new(error).color(Color32::RED)).clicked() {
318-
ui_state.serialization_error = None;
319-
}
320-
}
321321

322322
ui.separator();
323323
ui.hyperlink_to("Github", "https://github.com/vdrn/rust_graph");
@@ -343,8 +343,6 @@ pub fn side_panel<T: EvalexprFloat>(
343343
state.ctx.clear();
344344
init_builtins::<T>(&mut state.ctx);
345345

346-
// state.context_stash.lock().unwrap().clear();
347-
348346
prepare_entries(
349347
&mut state.graph_state.entries, &mut state.ctx, &mut ui_state.prepare_errors,
350348
);

src/entry/entry_processing.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,17 @@ fn prepare_entry<T: EvalexprFloat>(
108108

109109
let both_dirs_available =
110110
x_state.constants.iter().all(|c| !y_state.constants.contains(c));
111-
if !both_dirs_available
112-
&& point.drag.both_drag_dirs_available
113-
&& !matches!(point.drag.drag_type, PointDragType::NoDrag)
114-
{
115-
point.drag.drag_type = PointDragType::X;
116-
}
111+
// if !both_dirs_available
112+
// && point.drag.both_drag_dirs_available
113+
// && !matches!(point.drag.drag_type, PointDragType::NoDrag)
114+
// {
115+
// if debug {
116+
// println!("State x: {:?}", x_state);
117+
// println!("State y: {:?}", y_state);
118+
// println!("Overriding with X drag");
119+
// }
120+
// point.drag.drag_type = PointDragType::X;
121+
// }
117122
point.drag.both_drag_dirs_available = both_dirs_available;
118123

119124
match point.drag.drag_type {
@@ -134,7 +139,6 @@ fn prepare_entry<T: EvalexprFloat>(
134139
} else if let (Some(x_const), Some(y_const)) =
135140
(x_state.constants.first(), y_state.constants.first())
136141
{
137-
// todo:
138142
if x_const == y_const {
139143
if let Some(y_const) = y_state.constants.get(1) {
140144
point.drag.drag_point = Some(DragPoint::BothCoordConstants(
@@ -154,7 +158,6 @@ fn prepare_entry<T: EvalexprFloat>(
154158
Some(DragPoint::BothCoordConstants(istr(x_const), istr(y_const)));
155159
}
156160
} else if let Some(x_const) = x_state.constants.first()
157-
// && y_state.first_constant.is_none()
158161
{
159162
point.drag.drag_point = Some(DragPoint::XConstant(istr(x_const)));
160163
} else if let Some(y_const) = y_state.constants.first() {
@@ -310,6 +313,7 @@ fn prepare_constant<T: EvalexprFloat>(
310313
}
311314
Ok(())
312315
}
316+
#[derive(Debug)]
313317
struct NodeAnalysis<'a> {
314318
is_literal: bool,
315319
num_identifiers_and_special_ops: u32,
@@ -321,8 +325,8 @@ fn analyze_node<T: EvalexprFloat>(node: &FlatNode<T>) -> NodeAnalysis<'_> {
321325
let mut constants = SmallVec::new();
322326
let mut num_identifiers = 0;
323327

324-
for i in node.iter_variable_identifiers() {
325-
constants.push(i);
328+
for ident in node.iter_variable_identifiers() {
329+
constants.push(ident);
326330
is_literal = false;
327331
}
328332

src/entry/entry_ui.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ pub fn entry_ui<T: EvalexprFloat>(
9696
}
9797
if RESERVED_NAMES.contains(&entry.name.trim()) {
9898
result.error = Some(format!("{} is reserved name.", entry.name));
99-
// return;
10099
} else if !name_was_ok {
101100
result.parsed = true;
102101
}
@@ -255,7 +254,7 @@ fn entry_style<T: EvalexprFloat>(
255254
}
256255

257256
ui.separator();
258-
changed |= ui.checkbox(&mut style.show_points, "Show Not Draggable Points").clicked();
257+
changed |= ui.checkbox(&mut style.show_points, "Show Non Draggable Points").clicked();
259258

260259
egui::Sides::new().show(
261260
ui,

src/persistence.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,20 +357,20 @@ pub fn deserialize_from_url<T: EvalexprFloat>(
357357
// deserialize_from_json(decoded.as_bytes())
358358
}
359359

360-
pub fn deserialize_graph_state<T: EvalexprFloat>(name: String, ser: StateSerialized) -> GraphState<T> {
360+
pub fn deserialize_graph_state<T: EvalexprFloat>(name: String, ser: StateSerialized) -> Result<GraphState<T>,String> {
361361
let entries = deserialize_entries::<T>(ser.entries);
362362
let mut unique_ids = FxHashSet::default();
363363
let mut max_id = 0;
364364
for entry in entries.iter() {
365365
max_id = entry.id.max(max_id);
366366
if !unique_ids.insert(entry.id) {
367-
panic!("Duplicate id: {}", entry.id);
367+
return Err(format!("Error deserializing graph state: Duplicate id: {}", entry.id));
368368
}
369369
if let EntryType::Folder { entries } = &entry.ty {
370370
for sub_entry in entries.iter() {
371371
max_id = sub_entry.id.max(max_id);
372372
if !unique_ids.insert(sub_entry.id) {
373-
panic!("Duplicate id: {}", sub_entry.id);
373+
return Err(format!("Error deserializing graph state: Duplicate id: {}", sub_entry.id));
374374
}
375375
}
376376
}
@@ -379,14 +379,14 @@ pub fn deserialize_graph_state<T: EvalexprFloat>(name: String, ser: StateSeriali
379379
let max_id = max_id + 1;
380380

381381

382-
GraphState {
382+
Ok(GraphState {
383383
entries,
384384
saved_graph_config: ser.graph_config.clone(),
385385
current_graph_config: ser.graph_config,
386386
name,
387387
id_gen: IdGenerator::new(max_id),
388388
prev_plot_transform: None,
389-
}
389+
})
390390
}
391391
pub fn deserialize_entries<T: EvalexprFloat>(entries: Vec<EntrySerialized>) -> Vec<Entry<T>> {
392392
let mut result = Vec::new();
@@ -495,7 +495,7 @@ pub fn deserialize_graph_state_from_json<T: EvalexprFloat>(
495495
name: String, reader: &[u8],
496496
) -> Result<GraphState<T>, String> {
497497
let graph_state: StateSerialized = serde_json::from_slice(reader).map_err(|e| e.to_string())?;
498-
Ok(deserialize_graph_state(name, graph_state))
498+
deserialize_graph_state(name, graph_state)
499499
}
500500
// pub fn deserialize_from_url<T: EvalexprFloat>(url: &str) -> Result<Vec<Entry<T>>, String> {
501501
// }

0 commit comments

Comments
 (0)