diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-24 11:22:59 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-24 11:22:59 +0300 |
| commit | 871f7fa2b9a0fd5653adadfc9efb90fcbd2da203 (patch) | |
| tree | 1f9c21649bf437e9ef6678d7037d24617e71d736 | |
| parent | d35efdcdde0b6985473684f0b5668950a8477703 (diff) | |
rpn: replace panic with error returns in metric registry lookups
resolveMetric, coolMetric, and baseMetric all panicked when the
metric registry was missing expected entries. panic() violates Go
best practice (no panic except truly unrecoverable) and can crash the
REPL. resolveMetric is called for every arithmetic operation, making
this a high-impact surface.
Change all three functions to return (*Metric, error) instead of
panicking. Propagate errors through all callers:
- operations_metric.go: resolveMetric, coolMetric, baseMetric,
convertToBase, convertFromBase, resultMetricForMul, resultMetricForDiv, Convert
- operations_arithmetic.go: binaryMetricOp, Divide, Modulo
- operations_compare.go: compareValues
- operations_hyper.go: HyperAdd, HyperMultiply, HyperSubtract,
HyperDivide, HyperPower, HyperModulo, hyperLog
- operations_metric_cmd.go: MetricCompatible
| -rw-r--r-- | internal/rpn/operations_arithmetic.go | 65 | ||||
| -rw-r--r-- | internal/rpn/operations_compare.go | 10 | ||||
| -rw-r--r-- | internal/rpn/operations_hyper.go | 53 | ||||
| -rw-r--r-- | internal/rpn/operations_metric.go | 53 | ||||
| -rw-r--r-- | internal/rpn/operations_metric_cmd.go | 10 |
5 files changed, 143 insertions, 48 deletions
diff --git a/internal/rpn/operations_arithmetic.go b/internal/rpn/operations_arithmetic.go index ca32721..d0b0798 100644 --- a/internal/rpn/operations_arithmetic.go +++ b/internal/rpn/operations_arithmetic.go @@ -22,15 +22,21 @@ func (o *Operations) binaryMetricOp( op string, compatCheck func(*Metric, *Metric) error, compute func(float64, float64) float64, - resultMetricFn func(*MetricRegistry, *Metric, *Metric) *Metric, + resultMetricFn func(*MetricRegistry, *Metric, *Metric) (*Metric, error), ) error { a, b, err := popTwo(stack, op) if err != nil { return err } - aM := resolveMetric(o.metricRegistry, a) - bM := resolveMetric(o.metricRegistry, b) + aM, err := resolveMetric(o.metricRegistry, a) + if err != nil { + return buildError(op, err) + } + bM, err := resolveMetric(o.metricRegistry, b) + if err != nil { + return buildError(op, err) + } if compatCheck != nil { if err := compatCheck(aM, bM); err != nil { return err @@ -38,7 +44,10 @@ func (o *Operations) binaryMetricOp( } pm := o.GetPrefixMode() - resultMetric := resultMetricFn(o.metricRegistry, aM, bM) + resultMetric, err := resultMetricFn(o.metricRegistry, aM, bM) + if err != nil { + return buildError(op, err) + } aBase, err := convertToBase(o.metricRegistry, a, pm, resultMetric) if err != nil { return buildError(op, err) @@ -47,7 +56,10 @@ func (o *Operations) binaryMetricOp( if err != nil { return buildError(op, err) } - resultVal := convertFromBase(o.metricRegistry, compute(aBase, bBase), resultMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, compute(aBase, bBase), resultMetric, pm) + if err != nil { + return buildError(op, err) + } stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), resultMetric)) return nil @@ -64,7 +76,9 @@ func (o *Operations) Add(stack *Stack) error { return nil }, func(a, b float64) float64 { return a + b }, - compatibleMetric, + func(reg *MetricRegistry, aM, bM *Metric) (*Metric, error) { + return compatibleMetric(reg, aM, bM), nil + }, ) } @@ -79,7 +93,9 @@ func (o *Operations) Subtract(stack *Stack) error { return nil }, func(a, b float64) float64 { return a - b }, - compatibleMetric, + func(reg *MetricRegistry, aM, bM *Metric) (*Metric, error) { + return compatibleMetric(reg, aM, bM), nil + }, ) } @@ -108,11 +124,20 @@ func (o *Operations) Divide(stack *Stack) error { return buildError("/", fmt.Errorf("division by zero")) } - aM := resolveMetric(o.metricRegistry, a) - bM := resolveMetric(o.metricRegistry, b) + aM, err := resolveMetric(o.metricRegistry, a) + if err != nil { + return buildError("/", err) + } + bM, err := resolveMetric(o.metricRegistry, b) + if err != nil { + return buildError("/", err) + } pm := o.GetPrefixMode() - resultMetric := resultMetricForDiv(o.metricRegistry, aM, bM) + resultMetric, err := resultMetricForDiv(o.metricRegistry, aM, bM) + if err != nil { + return buildError("/", err) + } aBase, err := convertToBase(o.metricRegistry, a, pm, resultMetric) if err != nil { return buildError("/", err) @@ -121,7 +146,10 @@ func (o *Operations) Divide(stack *Stack) error { if err != nil { return buildError("/", err) } - resultVal := convertFromBase(o.metricRegistry, aBase/bBase, resultMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, aBase/bBase, resultMetric, pm) + if err != nil { + return buildError("/", err) + } stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), resultMetric)) return nil @@ -172,8 +200,14 @@ func (o *Operations) Modulo(stack *Stack) error { return buildError("%", fmt.Errorf("modulo by zero")) } - aM := resolveMetric(o.metricRegistry, a) - bM := resolveMetric(o.metricRegistry, b) + aM, err := resolveMetric(o.metricRegistry, a) + if err != nil { + return buildError("%", err) + } + bM, err := resolveMetric(o.metricRegistry, b) + if err != nil { + return buildError("%", err) + } if !categoriesCompatible(aM, bM) { return metricError("%", aM, bM) } @@ -188,7 +222,10 @@ func (o *Operations) Modulo(stack *Stack) error { if err != nil { return buildError("%", err) } - resultVal := convertFromBase(o.metricRegistry, math.Mod(aBase, bBase), resultMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, math.Mod(aBase, bBase), resultMetric, pm) + if err != nil { + return buildError("%", err) + } stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), resultMetric)) return nil diff --git a/internal/rpn/operations_compare.go b/internal/rpn/operations_compare.go index 81192e9..b21a00c 100644 --- a/internal/rpn/operations_compare.go +++ b/internal/rpn/operations_compare.go @@ -13,8 +13,14 @@ func compareValues(o *Operations, stack *Stack, op string, cmp func(float64, flo return err } - aM := resolveMetric(o.metricRegistry, a) - bM := resolveMetric(o.metricRegistry, b) + aM, err := resolveMetric(o.metricRegistry, a) + if err != nil { + return buildError(op, err) + } + bM, err := resolveMetric(o.metricRegistry, b) + if err != nil { + return buildError(op, err) + } if !categoriesCompatible(aM, bM) { return metricError(op, aM, bM) } diff --git a/internal/rpn/operations_hyper.go b/internal/rpn/operations_hyper.go index 19c9398..34b520b 100644 --- a/internal/rpn/operations_hyper.go +++ b/internal/rpn/operations_hyper.go @@ -23,7 +23,11 @@ func (o *Operations) HyperAdd(stack *Stack) error { // Resolve metrics for all values metrics := make([]*Metric, len(values)) for i, v := range values { - metrics[i] = resolveMetric(o.metricRegistry, v) + m, err := resolveMetric(o.metricRegistry, v) + if err != nil { + return buildError("[+]", err) + } + metrics[i] = m } // Validate all are compatible (all same category, or Cool absorbs) @@ -45,7 +49,10 @@ func (o *Operations) HyperAdd(stack *Stack) error { sum += base } - resultVal := convertFromBase(o.metricRegistry, sum, resultMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, sum, resultMetric, pm) + if err != nil { + return buildError("[+]", err) + } stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), resultMetric)) return nil @@ -72,7 +79,10 @@ func (o *Operations) HyperMultiply(stack *Stack) error { } } - cool := coolMetric(o.metricRegistry) + cool, err := coolMetric(o.metricRegistry) + if err != nil { + return buildError("[*]", err) + } stack.Push(NewNumberWithMetric(product, o.GetMode(), cool)) return nil } @@ -89,7 +99,11 @@ func (o *Operations) HyperSubtract(stack *Stack) error { // Resolve metrics for all values metrics := make([]*Metric, len(values)) for i, v := range values { - metrics[i] = resolveMetric(o.metricRegistry, v) + m, err := resolveMetric(o.metricRegistry, v) + if err != nil { + return buildError("[-]", err) + } + metrics[i] = m } // Validate all are compatible (all same category, or Cool absorbs) @@ -115,7 +129,10 @@ func (o *Operations) HyperSubtract(stack *Stack) error { result -= base } - resultVal := convertFromBase(o.metricRegistry, result, resultMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, result, resultMetric, pm) + if err != nil { + return buildError("[-]", err) + } stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), resultMetric)) return nil @@ -145,7 +162,10 @@ func (o *Operations) HyperDivide(stack *Stack) error { result /= val } - cool := coolMetric(o.metricRegistry) + cool, err := coolMetric(o.metricRegistry) + if err != nil { + return buildError("[/]", err) + } stack.Push(NewNumberWithMetric(result, o.GetMode(), cool)) return nil } @@ -171,7 +191,10 @@ func (o *Operations) HyperPower(stack *Stack) error { result = math.Pow(result, val) } - cool := coolMetric(o.metricRegistry) + cool, err := coolMetric(o.metricRegistry) + if err != nil { + return buildError("[^]", err) + } stack.Push(NewNumberWithMetric(result, o.GetMode(), cool)) return nil } @@ -188,7 +211,11 @@ func (o *Operations) HyperModulo(stack *Stack) error { // Resolve metrics for all values metrics := make([]*Metric, len(values)) for i, v := range values { - metrics[i] = resolveMetric(o.metricRegistry, v) + m, err := resolveMetric(o.metricRegistry, v) + if err != nil { + return buildError("[%]", err) + } + metrics[i] = m } // Validate all are compatible (all same category, or Cool absorbs) @@ -217,7 +244,10 @@ func (o *Operations) HyperModulo(stack *Stack) error { result = math.Mod(result, base) } - resultVal := convertFromBase(o.metricRegistry, result, resultMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, result, resultMetric, pm) + if err != nil { + return buildError("[%]", err) + } stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), resultMetric)) return nil @@ -243,7 +273,10 @@ func (o *Operations) hyperLog(stack *Stack, opName string, logFn func(float64) f result += logFn(val) } - cool := coolMetric(o.metricRegistry) + cool, err := coolMetric(o.metricRegistry) + if err != nil { + return buildError(opName, err) + } stack.Push(NewNumberWithMetric(result, o.GetMode(), cool)) return nil } diff --git a/internal/rpn/operations_metric.go b/internal/rpn/operations_metric.go index 4809ce1..decaad8 100644 --- a/internal/rpn/operations_metric.go +++ b/internal/rpn/operations_metric.go @@ -6,16 +6,16 @@ package rpn import "fmt" // resolveMetric returns the metric for a StackValue, defaulting to Cool if nil. -func resolveMetric(reg *MetricRegistry, n StackValue) *Metric { +func resolveMetric(reg *MetricRegistry, n StackValue) (*Metric, error) { m := n.Metric() if m == nil { var ok bool m, ok = reg.Find("Cool") if !ok { - panic("metric registry missing Cool metric") + return nil, fmt.Errorf("metric registry missing Cool metric") } } - return m + return m, nil } // validateCategories checks that all metrics belong to the same category. @@ -78,7 +78,10 @@ func convertToBase(reg *MetricRegistry, n StackValue, mode PrefixMode, resultMet if !ok { return 0, fmt.Errorf("convertToBase: value %q is not numeric", n) } - m := resolveMetric(reg, n) + m, err := resolveMetric(reg, n) + if err != nil { + return 0, err + } val, err := nv.Float64() if err != nil { return 0, fmt.Errorf("convertToBase: %w", err) @@ -92,23 +95,27 @@ func convertToBase(reg *MetricRegistry, n StackValue, mode PrefixMode, resultMet } // convertFromBase converts a base-unit value back to the given metric. -func convertFromBase(reg *MetricRegistry, baseVal float64, m *Metric, mode PrefixMode) float64 { +func convertFromBase(reg *MetricRegistry, baseVal float64, m *Metric, mode PrefixMode) (float64, error) { if m == nil { - m = baseMetric(reg, "Cool") + var err error + m, err = baseMetric(reg, "Cool") + if err != nil { + return 0, err + } } - return baseVal / m.Factor(mode) + return baseVal / m.Factor(mode), nil } // resultMetricForMul computes the resulting metric for multiplication. -func resultMetricForMul(reg *MetricRegistry, a, b *Metric) *Metric { +func resultMetricForMul(reg *MetricRegistry, a, b *Metric) (*Metric, error) { if a == nil || a.Category == Universal { if b == nil { return baseMetric(reg, "Cool") } - return b + return b, nil } if b == nil || b.Category == Universal { - return a + return a, nil } // Cross-category inference @@ -127,7 +134,7 @@ func resultMetricForMul(reg *MetricRegistry, a, b *Metric) *Metric { } // resultMetricForDiv computes the resulting metric for division. -func resultMetricForDiv(reg *MetricRegistry, a, b *Metric) *Metric { +func resultMetricForDiv(reg *MetricRegistry, a, b *Metric) (*Metric, error) { if a == nil && b == nil { return baseMetric(reg, "Cool") } @@ -135,7 +142,7 @@ func resultMetricForDiv(reg *MetricRegistry, a, b *Metric) *Metric { if a == nil { return baseMetric(reg, "Cool") } - return a + return a, nil } // When dividend is Cool (unitless) and divisor has a metric, // result should be Cool. E.g., 5 / 10km → 0.5 (Cool, not km). @@ -170,21 +177,21 @@ func metricError(op string, a, b *Metric) error { } // coolMetric returns the Cool metric from the registry. -func coolMetric(reg *MetricRegistry) *Metric { +func coolMetric(reg *MetricRegistry) (*Metric, error) { m, ok := reg.Find("Cool") if !ok { - panic("metric registry missing Cool metric") + return nil, fmt.Errorf("metric registry missing Cool metric") } - return m + return m, nil } // baseMetric looks up a base metric from the registry. -func baseMetric(reg *MetricRegistry, name string) *Metric { +func baseMetric(reg *MetricRegistry, name string) (*Metric, error) { m, ok := reg.Find(name) if !ok { - panic(fmt.Sprintf("metric registry missing base unit %q", name)) + return nil, fmt.Errorf("metric registry missing base unit %q", name) } - return m + return m, nil } // Convert converts a value from its current metric to a target metric. @@ -204,7 +211,10 @@ func (o *Operations) Convert(stack *Stack) error { } // 3. Get metrics targetMetric := target.Metric() - valueMetric := resolveMetric(o.metricRegistry, value) + valueMetric, err := resolveMetric(o.metricRegistry, value) + if err != nil { + return buildError("convert", err) + } // 4. Validate same category (or Cool absorbing) if !categoriesCompatible(valueMetric, targetMetric) { return metricError("convert", valueMetric, targetMetric) @@ -215,7 +225,10 @@ func (o *Operations) Convert(stack *Stack) error { if err != nil { return buildError("convert", err) } - resultVal := convertFromBase(o.metricRegistry, baseVal, targetMetric, pm) + resultVal, err := convertFromBase(o.metricRegistry, baseVal, targetMetric, pm) + if err != nil { + return buildError("convert", err) + } // 6. Push result with target metric stack.Push(NewNumberWithMetric(resultVal, o.GetMode(), targetMetric)) return nil diff --git a/internal/rpn/operations_metric_cmd.go b/internal/rpn/operations_metric_cmd.go index 31a2d2f..8e0e519 100644 --- a/internal/rpn/operations_metric_cmd.go +++ b/internal/rpn/operations_metric_cmd.go @@ -66,8 +66,14 @@ func (o *Operations) MetricCompatible(stack *Stack) (string, error) { vals := stack.Values() top := vals[len(vals)-1] second := vals[len(vals)-2] - mA := resolveMetric(o.metricRegistry, second) - mB := resolveMetric(o.metricRegistry, top) + mA, err := resolveMetric(o.metricRegistry, second) + if err != nil { + return "", buildError("metric compatible", err) + } + mB, err := resolveMetric(o.metricRegistry, top) + if err != nil { + return "", buildError("metric compatible", err) + } compatible := categoriesCompatible(mA, mB) result := fmt.Sprintf("%s (%s) and %s (%s): %v", mA.Name, mA.Category, mB.Name, mB.Category, compatible) |
