Smarter MIN, MAX, ABS macros

Use macros that explicitly avoid double-evaluation and can be used for any datatype, replacing `min`, `max`, `abs`, `fabs`, `labs`, and `FABS`.

Co-Authored-By: ejtagle <ejtagle@hotmail.com>

Marlin/src/HAL/HAL_DUE/usb/compiler.h

@@ -785,79 +785,6 @@ typedef struct
 
 //! @}
 
-
-/*! \name Mathematics
- *
- * The same considerations as for clz and ctz apply here but GCC does not
- * provide built-in functions to access the assembly instructions abs, min and
- * max and it does not produce them by itself in most cases, so two sets of
- * macros are defined here:
- *   - Abs, Min and Max to apply to constant expressions (values known at
- *     compile time);
- *   - abs, min and max to apply to non-constant expressions (values unknown at
- *     compile time), abs is found in stdlib.h.
- */
-//! @{
-
-/*! \brief Takes the absolute value of \a a.
- *
- * \param a Input value.
- *
- * \return Absolute value of \a a.
- *
- * \note More optimized if only used with values known at compile time.
- */
-#define Abs(a)              (((a) <  0 ) ? -(a) : (a))
-
-/*! \brief Takes the minimal value of \a a and \a b.
- *
- * \param a Input value.
- * \param b Input value.
- *
- * \return Minimal value of \a a and \a b.
- *
- * \note More optimized if only used with values known at compile time.
- */
-#define Min(a, b)           (((a) < (b)) ?  (a) : (b))
-
-/*! \brief Takes the maximal value of \a a and \a b.
- *
- * \param a Input value.
- * \param b Input value.
- *
- * \return Maximal value of \a a and \a b.
- *
- * \note More optimized if only used with values known at compile time.
- */
-#define Max(a, b)           (((a) > (b)) ?  (a) : (b))
-
-// abs() is already defined by stdlib.h
-
-/*! \brief Takes the minimal value of \a a and \a b.
- *
- * \param a Input value.
- * \param b Input value.
- *
- * \return Minimal value of \a a and \a b.
- *
- * \note More optimized if only used with values unknown at compile time.
- */
-#define min(a, b)   Min(a, b)
-
-/*! \brief Takes the maximal value of \a a and \a b.
- *
- * \param a Input value.
- * \param b Input value.
- *
- * \return Maximal value of \a a and \a b.
- *
- * \note More optimized if only used with values unknown at compile time.
- */
-#define max(a, b)   Max(a, b)
-
-//! @}
-
-
 /*! \brief Calls the routine at address \a addr.
  *
  * It generates a long call opcode.

Marlin/src/HAL/HAL_DUE/usb/uotghs_device_due.c

@@ -1904,7 +1904,7 @@ static void udd_ep_in_sent(udd_ep_id_t ep)
 		ptr_src = &ptr_job->buf[ptr_job->buf_cnt];
 		nb_remain = ptr_job->buf_size - ptr_job->buf_cnt;
 		// Fill a bank even if no data (ZLP)
-		nb_data = min(nb_remain, pkt_size);
+		nb_data = MIN(nb_remain, pkt_size);
 		// Modify job information
 		ptr_job->buf_cnt += nb_data;
 		ptr_job->buf_load = nb_data;

Marlin/src/HAL/HAL_DUE/usb/uotghs_device_due.h

@@ -290,7 +290,7 @@ extern "C" {
   //! Bounds given integer size to allowed range and rounds it up to the nearest
   //! available greater size, then applies register format of UOTGHS controller
   //! for endpoint size bit-field.
-#define udd_format_endpoint_size(size)            (32 - clz(((uint32_t)min(max(size, 8), 1024) << 1) - 1) - 1 - 3)
+#define udd_format_endpoint_size(size)            (32 - clz(((uint32_t)MIN(MAX(size, 8), 1024) << 1) - 1) - 1 - 3)
   //! Configures the selected endpoint size
 #define udd_configure_endpoint_size(ep, size)     (Wr_bitfield(UOTGHS_ARRAY(UOTGHS_DEVEPTCFG[0], ep), UOTGHS_DEVEPTCFG_EPSIZE_Msk, udd_format_endpoint_size(size)))
   //! Gets the configured selected endpoint size

Marlin/src/HAL/HAL_LPC1768/SoftwareSPI.cpp

@@ -84,7 +84,7 @@ void swSpiBegin(const pin_t sck_pin, const pin_t miso_pin, const pin_t mosi_pin)
 uint8_t swSpiInit(const uint8_t spiRate, const pin_t sck_pin, const pin_t mosi_pin) {
   WRITE(mosi_pin, HIGH);
   WRITE(sck_pin, LOW);
-  return (SystemCoreClock == 120000000 ? 44 : 38) / POW(2, 6 - min(spiRate, 6));
+  return (SystemCoreClock == 120000000 ? 44 : 38) / POW(2, 6 - MIN(spiRate, 6));
 }
 
 #endif // TARGET_LPC1768

Marlin/src/HAL/HAL_LPC1768/include/Arduino.h

@@ -50,21 +50,18 @@ typedef uint8_t byte;
 #define PSTR(v) (v)
 #define PGM_P const char *
 
+// Used for libraries, preprocessor, and constants
 #define min(a,b) ((a)<(b)?(a):(b))
 #define max(a,b) ((a)>(b)?(a):(b))
 #define abs(x) ((x)>0?(x):-(x))
+
 #ifndef isnan
   #define isnan std::isnan
 #endif
 #ifndef isinf
   #define isinf std::isinf
 #endif
 
-//not constexpr until c++14
-//#define max(v1, v2) std::max((int)v1,(int)v2)
-//#define min(v1, v2) std::min((int)v1,(int)v2)
-//#define abs(v) std::abs(v)
-
 #define sq(v) ((v) * (v))
 #define square(v) sq(v)
 #define constrain(value, arg_min, arg_max) ((value) < (arg_min) ? (arg_min) :((value) > (arg_max) ? (arg_max) : (value)))

Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.cpp

@@ -121,7 +121,7 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
       timer_set_count(STEP_TIMER_DEV, 0);
       timer_set_prescaler(STEP_TIMER_DEV, (uint16)(STEPPER_TIMER_PRESCALE - 1));
       timer_set_reload(STEP_TIMER_DEV, 0xFFFF);
-      timer_set_compare(STEP_TIMER_DEV, STEP_TIMER_CHAN, min(HAL_TIMER_TYPE_MAX, (HAL_STEPPER_TIMER_RATE / frequency)));
+      timer_set_compare(STEP_TIMER_DEV, STEP_TIMER_CHAN, MIN(HAL_TIMER_TYPE_MAX, (HAL_STEPPER_TIMER_RATE / frequency)));
       timer_attach_interrupt(STEP_TIMER_DEV, STEP_TIMER_CHAN, stepTC_Handler);
       nvic_irq_set_priority(irq_num, 1);
       timer_generate_update(STEP_TIMER_DEV);
@@ -132,7 +132,7 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
       timer_set_count(TEMP_TIMER_DEV, 0);
       timer_set_prescaler(TEMP_TIMER_DEV, (uint16)(TEMP_TIMER_PRESCALE - 1));
       timer_set_reload(TEMP_TIMER_DEV, 0xFFFF);
-      timer_set_compare(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, min(HAL_TIMER_TYPE_MAX, ((F_CPU / TEMP_TIMER_PRESCALE) / frequency)));
+      timer_set_compare(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, MIN(HAL_TIMER_TYPE_MAX, ((F_CPU / TEMP_TIMER_PRESCALE) / frequency)));
       timer_attach_interrupt(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, tempTC_Handler);
       nvic_irq_set_priority(irq_num, 2);
       timer_generate_update(TEMP_TIMER_DEV);

Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.h

@@ -130,7 +130,7 @@ bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
  */
 
 FORCE_INLINE static void HAL_timer_set_compare(const uint8_t timer_num, const hal_timer_t compare) {
-  //compare = min(compare, HAL_TIMER_TYPE_MAX);
+  //compare = MIN(compare, HAL_TIMER_TYPE_MAX);
   switch (timer_num) {
   case STEP_TIMER_NUM:
     timer_set_compare(STEP_TIMER_DEV, STEP_TIMER_CHAN, compare);

Marlin/src/HAL/HAL_STM32F7/TMC2660.cpp

@@ -237,7 +237,7 @@ unsigned int TMC26XStepper::getSpeed(void) { return this->speed; }
  */
 char TMC26XStepper::step(int steps_to_move) {
   if (this->steps_left == 0) {
-    this->steps_left = abs(steps_to_move);  // how many steps to take
+    this->steps_left = ABS(steps_to_move);  // how many steps to take
 
     // determine direction based on whether steps_to_move is + or -:
     if (steps_to_move > 0)
@@ -257,7 +257,7 @@ char TMC26XStepper::move(void) {
 
     // rem if (time >= this->next_step_time) {
 
-    if (abs(time - this->last_step_time) > this->step_delay) {
+    if (ABS(time - this->last_step_time) > this->step_delay) {
       // increment or decrement the step number,
       // depending on direction:
       if (this->direction == 1)

Marlin/src/HAL/servo.cpp

@@ -99,7 +99,7 @@ int8_t Servo::attach(const int pin, const int min, const int max) {
   if (pin > 0) servo_info[this->servoIndex].Pin.nbr = pin;
   pinMode(servo_info[this->servoIndex].Pin.nbr, OUTPUT); // set servo pin to output
 
-  // todo min/max check: abs(min - MIN_PULSE_WIDTH) /4 < 128
+  // todo min/max check: ABS(min - MIN_PULSE_WIDTH) /4 < 128
   this->min = (MIN_PULSE_WIDTH - min) / 4; //resolution of min/max is 4 uS
   this->max = (MAX_PULSE_WIDTH - max) / 4;
 

Marlin/src/core/macros.h

@@ -113,7 +113,7 @@
 #define DECIMAL_SIGNED(a) (DECIMAL(a) || (a) == '-' || (a) == '+')
 #define COUNT(a) (sizeof(a)/sizeof(*a))
 #define ZERO(a) memset(a,0,sizeof(a))
-#define COPY(a,b) memcpy(a,b,min(sizeof(a),sizeof(b)))
+#define COPY(a,b) memcpy(a,b,MIN(sizeof(a),sizeof(b)))
 
 // Macros for initializing arrays
 #define ARRAY_6(v1, v2, v3, v4, v5, v6, ...) { v1, v2, v3, v4, v5, v6 }
@@ -164,12 +164,48 @@
 
 #define CEILING(x,y) (((x) + (y) - 1) / (y))
 
-#define MIN3(a, b, c)       min(min(a, b), c)
-#define MIN4(a, b, c, d)    min(MIN3(a, b, c), d)
-#define MIN5(a, b, c, d, e) min(MIN4(a, b, c, d), e)
-#define MAX3(a, b, c)       max(max(a, b), c)
-#define MAX4(a, b, c, d)    max(MAX3(a, b, c), d)
-#define MAX5(a, b, c, d, e) max(MAX4(a, b, c, d), e)
+// Avoid double evaluation of arguments on MIN/MAX/ABS
+#undef MIN
+#undef MAX
+#undef ABS
+#ifdef __cplusplus
+
+  // C++11 solution that is standards compliant. Return type is deduced automatically
+  template <class L, class R> static inline constexpr auto MIN(const L lhs, const R rhs) -> decltype(lhs + rhs) {
+    return lhs < rhs ? lhs : rhs;
+  }
+  template <class L, class R> static inline constexpr auto MAX(const L lhs, const R rhs) -> decltype(lhs + rhs){
+    return lhs > rhs ? lhs : rhs;
+  }
+  template <class T> static inline constexpr const T ABS(const T v) {
+    return v >= 0 ? v : -v;
+  }
+#else
+
+  // Using GCC extensions, but Travis GCC version does not like it and gives
+  //  "error: statement-expressions are not allowed outside functions nor in template-argument lists"
+  #define MIN(a, b) \
+    ({__typeof__(a) _a = (a); \
+      __typeof__(b) _b = (b); \
+      _a < _b ? _a : _b;})
+
+  #define MAX(a, b) \
+    ({__typeof__(a) _a = (a); \
+      __typeof__(b) _b = (b); \
+      _a > _b ? _a : _b;})
+
+  #define ABS(a) \
+    ({__typeof__(a) _a = (a); \
+      _a >= 0 ? _a : -_a;})
+
+#endif
+
+#define MIN3(a, b, c)       MIN(MIN(a, b), c)
+#define MIN4(a, b, c, d)    MIN(MIN3(a, b, c), d)
+#define MIN5(a, b, c, d, e) MIN(MIN4(a, b, c, d), e)
+#define MAX3(a, b, c)       MAX(MAX(a, b), c)
+#define MAX4(a, b, c, d)    MAX(MAX3(a, b, c), d)
+#define MAX5(a, b, c, d, e) MAX(MAX4(a, b, c, d), e)
 
 #define UNEAR_ZERO(x) ((x) < 0.000001)
 #define NEAR_ZERO(x) WITHIN(x, -0.000001, 0.000001)
@@ -182,7 +218,6 @@
 // Maths macros that can be overridden by HAL
 //
 #define ATAN2(y, x) atan2(y, x)
-#define FABS(x)     fabs(x)
 #define POW(x, y)   pow(x, y)
 #define SQRT(x)     sqrt(x)
 #define CEIL(x)     ceil(x)

Marlin/src/feature/I2CPositionEncoder.cpp

@@ -134,7 +134,7 @@ void I2CPositionEncoder::update() {
 
     #ifdef I2CPE_EC_THRESH_PROPORTIONAL
       const millis_t deltaTime = positionTime - lastPositionTime;
-      const uint32_t distance = abs(position - lastPosition),
+      const uint32_t distance = ABS(position - lastPosition),
                      speed = distance / deltaTime;
       const float threshold = constrain((speed / 50), 1, 50) * ecThreshold;
     #else
@@ -150,7 +150,7 @@ void I2CPositionEncoder::update() {
 
       LOOP_L_N(i, I2CPE_ERR_ARRAY_SIZE) {
         sum += err[i];
-        if (i) diffSum += abs(err[i-1] - err[i]);
+        if (i) diffSum += ABS(err[i-1] - err[i]);
       }
 
       const int32_t error = int32_t(sum / (I2CPE_ERR_ARRAY_SIZE + 1)); //calculate average for error
@@ -163,7 +163,7 @@ void I2CPositionEncoder::update() {
     //SERIAL_ECHOLN(error);
 
     #ifdef I2CPE_ERR_THRESH_ABORT
-      if (labs(error) > I2CPE_ERR_THRESH_ABORT * planner.axis_steps_per_mm[encoderAxis]) {
+      if (ABS(error) > I2CPE_ERR_THRESH_ABORT * planner.axis_steps_per_mm[encoderAxis]) {
         //kill("Significant Error");
         SERIAL_ECHOPGM("Axis error greater than set threshold, aborting!");
         SERIAL_ECHOLN(error);
@@ -175,8 +175,8 @@ void I2CPositionEncoder::update() {
       if (errIdx == 0) {
         // In order to correct for "error" but avoid correcting for noise and non-skips
         // it must be > threshold and have a difference average of < 10 and be < 2000 steps
-        if (labs(error) > threshold * planner.axis_steps_per_mm[encoderAxis] &&
-            diffSum < 10 * (I2CPE_ERR_ARRAY_SIZE - 1) && labs(error) < 2000) { // Check for persistent error (skip)
+        if (ABS(error) > threshold * planner.axis_steps_per_mm[encoderAxis] &&
+            diffSum < 10 * (I2CPE_ERR_ARRAY_SIZE - 1) && ABS(error) < 2000) { // Check for persistent error (skip)
           errPrst[errPrstIdx++] = error; // Error must persist for I2CPE_ERR_PRST_ARRAY_SIZE error cycles. This also serves to improve the average accuracy
           if (errPrstIdx >= I2CPE_ERR_PRST_ARRAY_SIZE) {
             float sumP = 0;
@@ -193,14 +193,14 @@ void I2CPositionEncoder::update() {
           errPrstIdx = 0;
       }
     #else
-      if (labs(error) > threshold * planner.axis_steps_per_mm[encoderAxis]) {
+      if (ABS(error) > threshold * planner.axis_steps_per_mm[encoderAxis]) {
         //SERIAL_ECHOLN(error);
         //SERIAL_ECHOLN(position);
         thermalManager.babystepsTodo[encoderAxis] = -LROUND(error / 2);
       }
     #endif
 
-    if (labs(error) > I2CPE_ERR_CNT_THRESH * planner.axis_steps_per_mm[encoderAxis]) {
+    if (ABS(error) > I2CPE_ERR_CNT_THRESH * planner.axis_steps_per_mm[encoderAxis]) {
       const millis_t ms = millis();
       if (ELAPSED(ms, nextErrorCountTime)) {
         SERIAL_ECHOPAIR("Large error on ", axis_codes[encoderAxis]);
@@ -258,7 +258,7 @@ float I2CPositionEncoder::get_axis_error_mm(const bool report) {
   actual = mm_from_count(position);
   error = actual - target;
 
-  if (labs(error) > 10000) error = 0; // ?
+  if (ABS(error) > 10000) error = 0; // ?
 
   if (report) {
     SERIAL_ECHO(axis_codes[encoderAxis]);
@@ -293,7 +293,7 @@ int32_t I2CPositionEncoder::get_axis_error_steps(const bool report) {
           error = (encoderCountInStepperTicksScaled - target);
 
   //suppress discontinuities (might be caused by bad I2C readings...?)
-  bool suppressOutput = (labs(error - errorPrev) > 100);
+  bool suppressOutput = (ABS(error - errorPrev) > 100);
 
   if (report) {
     SERIAL_ECHO(axis_codes[encoderAxis]);
@@ -435,7 +435,7 @@ void I2CPositionEncoder::calibrate_steps_mm(const uint8_t iter) {
     delay(250);
     stopCount = get_position();
 
-    travelledDistance = mm_from_count(abs(stopCount - startCount));
+    travelledDistance = mm_from_count(ABS(stopCount - startCount));
 
     SERIAL_ECHOPAIR("Attempted to travel: ", travelDistance);
     SERIAL_ECHOLNPGM("mm.");

Marlin/src/feature/Max7219_Debug_LEDs.cpp

@@ -347,8 +347,8 @@ void Max7219_idle_tasks() {
       NOMORE(current_depth, 16);        // if the BLOCK_BUFFER_SIZE is greater than 16, two lines
                                         // of LEDs is enough to see if the buffer is draining
 
-      const uint8_t st = min(current_depth, last_depth),
-                    en = max(current_depth, last_depth);
+      const uint8_t st = MIN(current_depth, last_depth),
+                    en = MAX(current_depth, last_depth);
       if (current_depth < last_depth)
         for (uint8_t i = st; i <= en; i++)   // clear the highest order LEDs
           Max7219_LED_Off(MAX7219_DEBUG_STEPPER_QUEUE + (i & 1), i / 2);

Marlin/src/feature/bedlevel/abl/abl.cpp

@@ -295,7 +295,7 @@ float bilinear_z_offset(const float raw[XYZ]) {
     #endif
 
     gridx = gx;
-    nextx = min(gridx + 1, ABL_BG_POINTS_X - 1);
+    nextx = MIN(gridx + 1, ABL_BG_POINTS_X - 1);
   }
 
   if (last_y != ry || last_gridx != gridx) {
@@ -312,7 +312,7 @@ float bilinear_z_offset(const float raw[XYZ]) {
       #endif
 
       gridy = gy;
-      nexty = min(gridy + 1, ABL_BG_POINTS_Y - 1);
+      nexty = MIN(gridy + 1, ABL_BG_POINTS_Y - 1);
     }
 
     if (last_gridx != gridx || last_gridy != gridy) {
@@ -336,7 +336,7 @@ float bilinear_z_offset(const float raw[XYZ]) {
 
   /*
   static float last_offset = 0;
-  if (FABS(last_offset - offset) > 0.2) {
+  if (ABS(last_offset - offset) > 0.2) {
     SERIAL_ECHOPGM("Sudden Shift at ");
     SERIAL_ECHOPAIR("x=", rx);
     SERIAL_ECHOPAIR(" / ", bilinear_grid_spacing[X_AXIS]);
@@ -389,7 +389,7 @@ float bilinear_z_offset(const float raw[XYZ]) {
     #define LINE_SEGMENT_END(A) (current_position[A ##_AXIS] + (destination[A ##_AXIS] - current_position[A ##_AXIS]) * normalized_dist)
 
     float normalized_dist, end[XYZE];
-    const int8_t gcx = max(cx1, cx2), gcy = max(cy1, cy2);
+    const int8_t gcx = MAX(cx1, cx2), gcy = MAX(cy1, cy2);
 
     // Crosses on the X and not already split on this X?
     // The x_splits flags are insurance against rounding errors.

Marlin/src/feature/bedlevel/mbl/mesh_bed_leveling.cpp

@@ -76,7 +76,7 @@
       #define MBL_SEGMENT_END(A) (current_position[A ##_AXIS] + (destination[A ##_AXIS] - current_position[A ##_AXIS]) * normalized_dist)
 
       float normalized_dist, end[XYZE];
-      const int8_t gcx = max(cx1, cx2), gcy = max(cy1, cy2);
+      const int8_t gcx = MAX(cx1, cx2), gcy = MAX(cy1, cy2);
 
       // Crosses on the X and not already split on this X?
       // The x_splits flags are insurance against rounding errors.