8
\$\begingroup\$

The simplest way one can achieve this is to have separate functions for each type with a suffix to differentiate between them like the following:

max_i();  // int
max_ui(); // unsigned int
// So on for other types.

But that becomes too cumbersome to remember. One could instead opt for function-like macros:

#define MAX(a, b)    ((a) > (b) ? (a) : (b))
#define MIN(a, b)    ((a) < (b) ? (a) : (b))

But these have the disadvantage that arguments are evaluated twice. This can cause some trouble in the following calls:

MAX(rand(), *p++)
MIN(--a, b++)

But a comment preceding the macros:

/* XXX: Do not pass in arguments with side-effects to these macros. */

takes the burden away from the implementer, and shifts it onto the developer (assuming they were different people).

But these macros are still frowned upon. One way in which the double evaluation part can be avoided is by using C23's auto and GNU C's compound expression statement (supported by Clang and GCC, I am unaware about others):

#if defined(__GNUC__) || defined(__clang__)
    #define MAX(a, b) ({    \
        auto x = (a);       \
        auto y = (b);       \
        x > y ? x : y; })

    #define MIN(a, b) ({    \
        auto x = (a);       \
        auto y = (b);       \
        x < y ? x : y; })
#endif

These evaluate their arguments only once. But compound expression statement is not part of ISO C.

In which case we're only left with C11's _Generic:

#define MAX(a, b)    _Generic((a) + (b),                      \
                        signed char:                max_sc,   \
                        unsigned char:              max_uc,   \
                        char:                       max_c,    \
                        short int:                  max_si,   \
                        unsigned short int:         max_usi,  \
                        int:                        max_i,    \
                        unsigned int:               max_ui,   \
                        long int:                   max_li,   \
                        unsigned long int:          max_uli,  \
                        long long int:              max_lli,  \
                        unsigned long long int:     max_lli,  \
                        float:                      max_f,    \
                        double:                     max_d,    \
                        long double:                max_ld)(a, b)

#define MIN(a, b)    _Generic((a) + (b),                      \
                        signed char:                min_sc,   \
                        unsigned char:              min_uc,   \
                        char:                       min_c,    \
                        short int:                  min_si,   \
                        unsigned short int:         min_usi,  \
                        int:                        min_i,    \
                        unsigned int:               min_ui,   \
                        long int:                   min_li,   \
                        unsigned long int:          min_uli,  \
                        long long int:              min_lli,  \
                        unsigned long long int:     min_ulli, \
                        float:                      min_f,    \
                        double:                     min_d,    \
                        long double:                min_ld)(a, b)

/* Or provide only the declarations here and have a corresponding source file
   with all the definitions. */
#define gen_max(type, suffix) \
    [[gnu::always_inline, gnu::const]] static inline max_##suffix(type a, type b) \
    { return a > b ? a : b; } 

#define gen_min(type, suffix) \
    [[gnu::always_inline, gnu::const]] static inline min_##suffix(type a, type b) \
    { return a > b ? a : b; } 

gen_max(signed char, sc)
gen_max(unsigned char, uc)
gen_max(char, c)
gen_max(short int, si)
gen_max(unsigned short int, usi)
gen_max(int, i)
gen_max(unsigned int, ui)
gen_max(long int, li)
gen_max(unsigned long int, uli)
gen_max(long long int, lli)
gen_max(unsigned long long int, ulli)
gen_max(float, f)
gen_max(double, d)
gen_max(long double, ld)

gen_min(signed char, sc)
gen_min(unsigned char, uc)
gen_min(char, c)
gen_min(short int, si)
gen_min(unsigned short int, usi)
gen_min(int, i)
gen_min(unsigned int, ui)
gen_min(long int, li)
gen_min(unsigned long int, uli)
gen_min(long long int, lli)
gen_min(unsigned long long int, ulli)
gen_min(float, f)
gen_min(double, d)
gen_min(long double, ld)

This is both safe (doesn't evaluate the arguments twice), and Standard C code (unlike the second option).

Review Request:

In the third implementation, I have (a) + (b) as the controlling expression of _Generic, which would invoke undefined behaviour on signed integer overflow. How can I avoid that?

Would it be a good idea to merge these 3 implementations into a single source file, with this structure:

If __GNUC__ or __clang__ is defined, use the compound expression statement.
Else use C11's _Generic.

Unconditionally use MIN/MAX with a comment to warn against side-effects. 
And perhaps name the previous ones MIN_SAFE/MIN_MAX.

? Do you see any other problems with the code, or difference between the implementations that I am overlooking?


Follow-ups:

\$\endgroup\$
6
  • \$\begingroup\$ typeof(a) could be used instead of auto too. \$\endgroup\$
    – Harith
    Commented May 26 at 18:01
  • 1
    \$\begingroup\$ With _Generic((a) + (b), the type will never be narrower than int. \$\endgroup\$ Commented May 27 at 21:14
  • \$\begingroup\$ @chux-ReinstateMonica Yes, the integer promotion rules. I don't see the use of keeping the 3 char types and signed short int and unsigned short int then. \$\endgroup\$
    – Harith
    Commented May 27 at 23:06
  • \$\begingroup\$ Harith, aside from missing signed char, code is also missing _Bool from the primative type list, yet with 0*(a) + 0*(b), that can be ignored too. \$\endgroup\$ Commented May 28 at 0:23
  • 1
    \$\begingroup\$ I was recalling some other similar post and wrongly applied that here. \$\endgroup\$ Commented May 28 at 1:31

4 Answers 4

3
\$\begingroup\$

Let's go crazy and make N macros and N*N functions.

#define MAX_SC(a, b)    _Generic((b),     \
                        signed char:  max_scsc,   \
                        unsigned char:max_scuc,  \
                        // many others
                        long double: max_scld)((a), (b))

#define MAX_UC(a, b)    _Generic((b),     \
                        signed char:  max_ucsc,   \
                        unsigned char:max_ucuc,  \
                        // many others
                        long double: max_ucld)((a), (b))

// Many others MAX_..(a, b)

#define MAX(a, b)    _Generic((a),     \
                        signed char:  MAX_SC,   \
                        unsigned char:MAX_UC,  \
                        // many others
                        long double: MAX_LD)((a), (b))

Now the max of INT_MIN, UINT_MAX will be UINT_MAX
and the min of INT_MIN, UINT_MAX will be INT_MIN.

unsigned max_uii_(unsigned a, int b) {
  return b < 0 ? a : (a > (unsigned) b ? a : (unsigned) b);
}

int min_uii_(unsigned a, int b) {
  return (a > INT_MAX) ? b : ((int) a < b ? (int) a : b);
}


Mixed FP and integer types remain a problem.
\$\endgroup\$
7
  • \$\begingroup\$ Perhaps don't support FP because fmax()/fmin() exist? \$\endgroup\$
    – Harith
    Commented May 27 at 21:25
  • \$\begingroup\$ @Harith Its a type issue: example : the minimum of -0x37777777, nextafterf(-powf(2,31),0)? is INT_MIN+1, but that is not exactly representable as a float. Note" fmax()/fmin() exist since C99. \$\endgroup\$ Commented May 27 at 21:36
  • \$\begingroup\$ 14 macros and 196 functions for MAX(), 14 macros and 196 macros for MIN(). Nay, I'd fall back to the lovely one-liners. \$\endgroup\$
    – Harith
    Commented May 28 at 13:25
  • \$\begingroup\$ @Harith Mixed types narrower that int can use max_i_i_(). For mixed types like int, unsigned, the _Generic() could not resolve and lead to compiler failure. This failure may be preferable than MAX(-1, UINTMAX - 1u) resulting in UINTMAX. With this approach only about 8 MAX_() and 8 functions macros needed. \$\endgroup\$ Commented May 28 at 17:10
  • 1
    \$\begingroup\$ @Harith We could use _Generic((b) + 0) to eliminate the need for char, signed char, unsigned char, signed short, unsigned short cases. \$\endgroup\$ Commented Jun 4 at 0:10
6
\$\begingroup\$

The compound expression statement would restrict the scope of x and y, but there may still be conflicts with variable names that occur within the expressions that are substituted for a and b. For instance, if MAX() is called like this:

MAX(x++, y + 1);

The first statement of the compound statement expression would become:

auto x = (x++);

and the second statement would become:

auto y = (y + 1);

GCC's documentation suggests prefixing the identifiers with underscores to avoid these conflicts. Though I wouldn't second that, since identifiers prefixed with underscores are reserved.

You could use a temp/tmp suffix or prefix, or something similar.


Edit: I have found a better mechanism for generating a unique name:

We are going to use the C Preprocessor to help us, and cdecl 3.0, which supports nearly all C and C++ Standards and extensions, to expand the macros:

$ cdecl
cdecl> #define CONCAT2_EXPAND(a, b)    a ## b
cdecl> #define CONCAT2(a, b)           CONCAT2_EXPAND(a, b)
cdecl> #define UNIQUE_NAME(prefix)     CONCAT2(CONCAT2(prefix, _), __LINE__)
cdecl> expand UNIQUE_NAME(var)
UNIQUE_NAME(var) => CONCAT2(CONCAT2(prefix, _), __LINE__)
| prefix => var
UNIQUE_NAME(var) => CONCAT2(CONCAT2(var, _), __LINE__)
| CONCAT2(CONCAT2(var, _), __LINE__) => CONCAT2_EXPAND(a, b)
| | a => CONCAT2(var, _)
| | | CONCAT2(var, _) => CONCAT2_EXPAND(a, b)
| | | | a => var
| | | | b => _
| | | CONCAT2(var, _) => CONCAT2_EXPAND(var, _)
| | | | CONCAT2_EXPAND(var, _) => a ## b
| | | | CONCAT2_EXPAND(var, _) => var ## _
| | | | CONCAT2_EXPAND(var, _) => var_
| | | CONCAT2(var, _) => var_
| | a => var_
| | b => __LINE__
| | | __LINE__ => 42
| | b => 42
| CONCAT2(var_, 42) => CONCAT2_EXPAND(var_, 42)
| | CONCAT2_EXPAND(var_, 42) => a ## b
| | CONCAT2_EXPAND(var_, 42) => var_ ## 42
| | CONCAT2_EXPAND(var_, 42) => var_42
| CONCAT2(var_, 42) => var_42
UNIQUE_NAME(var) => var_42
cdecl> 

CONCAT2() simply concatenates its arguments. CONCAT2_EXPAND() is required because the ## operator does not expand its operands. UNIQUE_NAME() concatenates the prefix passed to it with the current line number.

The expand command above shows the complete working (the actual output is very colorful. Unfortunately, I can not replicate it here). This is likely to generate quite a unique identifier.

I found this at Handy C/C++ Preprocessor Macros. This blog is written by the same person who wrote v3 of cdecl. cdecl's codebase makes much use of these macros as well. See the blog for more preprocessor tricks.

None of these function-like macros take care of mixed types:

If MAX() is passed a signed type and an unsigned type, the signed type is likely going to be very converted to a very large unsigned type. The current 3 versions of MAX() would only work properly for types with same signs.

For long and int, good. For unsigned and unsigned long, good.

For int and unsigned, not good. For int and unsigned long, not good.

Overflow is not a problem:

As @Jonathan Leffer states in the comments of this StackOverflow question: Understanding C11's _Generic:

Note that §6.5.1.1 Generic selection ¶3 says: The controlling expression of a generic selection is not evaluated.. Since it isn't evaluated, it can't overflow. It makes sense. The controlling expression determines the type at compile time; overflow happens at run-time.

So (a) + (b) is correct, except that as @chux mentions in the comments:

With _Generic((a) + (b), the type will never be narrower than int.

\$\endgroup\$
9
  • 3
    \$\begingroup\$ To avoid conflict in macros, I tend to suffix them with an underscore and a randomly chosen alphanumeric suffix. Verbose, but when life deals you lemons... \$\endgroup\$ Commented May 27 at 7:38
  • \$\begingroup\$ My apologies if the edit sounds more like a promotion, I have just become very fond of the program. :) \$\endgroup\$
    – Harith
    Commented Jun 6 at 17:24
  • 1
    \$\begingroup\$ That's only if the prefix is user-facing, though. If UNIQUE_NAME is used within a macro and that macro doesn't expose prefix (but picks one), then using the macro twice on the same line (possibly nested, as in MAX(a, MAX(b, c))) will cause different variables to end with the same unique name. \$\endgroup\$ Commented Jun 7 at 10:09
  • 1
    \$\begingroup\$ I'm less far-sighted and more... experienced :'( I can't think of anything else, I mostly used C++ so I was able to mostly stay away from macros in general, just "had" to use them to implement proper logging and introspection. \$\endgroup\$ Commented Jun 7 at 16:09
  • 1
    \$\begingroup\$ Well, that's interesting. I hadn't realized this was a point of divergence between C and C++. There is still the risk of C code being included in a C++ project, so for small functions I'd recommend static inline, but for a pure C project, it sure makes things easier. \$\endgroup\$ Commented Jun 8 at 9:31
4
\$\begingroup\$

UB

(a) + (b)

which would invoke undefined behaviour on signed integer overflow.

Well sure, integer addition can overflow.

But we have operations that don't, right? Like all of the bitwise operations. XOR seems a fair candidate.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ You were right that {} would ensure that the local variables are only visible in that scope. But there's another problem. There may still be conflicts with variable names that occur within the expressions that are substituted for a and b. For instance, if I call MAX() like this: MAX(x++, y + 1), the first statement would become: auto x = (x++);, and the second statement would become: auto y = (y + 1);. GCC's documentation suggests prefixing the identifiers with underscores to avoid these conflicts. \$\endgroup\$
    – Harith
    Commented May 26 at 21:04
  • 3
    \$\begingroup\$ Yup, it's a generic issue with macro expansion. The scheme / lisp community refers to it as hygienic macros, and has a few ways of addressing it. These include special parser syntax, and (gensym), which generates a brand new guaranteed unique symbol for use in the expansion. \$\endgroup\$
    – J_H
    Commented May 26 at 21:13
  • \$\begingroup\$ Bitwise operations don't work for floats. \$\endgroup\$ Commented May 27 at 15:01
  • \$\begingroup\$ Instead of (a) + (b), perhaps 0*(a) + 0*(b) to get the common type and avoid overflow? \$\endgroup\$ Commented May 27 at 21:02
  • 4
    \$\begingroup\$ Multiply by zero is brilliant, yes, do that. \$\endgroup\$
    – J_H
    Commented May 27 at 21:03
3
\$\begingroup\$

Bug With Floating-Point NaN

The a > b ? a : b check will fail when b is NaN and a is not, since comparison with NaN is always false. The expected behavior is to return a. Use fmaximum_num, fmaximum_numf or fmaximum_numl for these types. (Unlike the fmax family, these are defined to handle NaN and comparisons between +0 and -0 correctly.)

Prior to C23, there is theoretically another issue that could come up with the sign-and-magnitude representation of -0. In C23, all signed integral types are two’s-complement.

\$\endgroup\$
3
  • \$\begingroup\$ "since comparison with NaN is always false." --> that is an IEEE spec that C is not obliged to follow - although that result is common. "The a > b ? a : b check will fail when b is NaN and a is not," could fail or pass in C. \$\endgroup\$ Commented May 27 at 21:46
  • \$\begingroup\$ "The expected behavior is to return a" --> C2X discusses that in fmax() when b is a quiet NAN and appears silent when both a,b are quiet, yet different NANs. It is also quiet on fmax(+0.0,-0,0) and fmax(-0.0,+0,0) - one of those get returned? I do not see C specifying which. \$\endgroup\$ Commented May 27 at 21:49
  • \$\begingroup\$ @chux-ReinstateMonica Looking it up, in C23, there is a separate fmaximum_num family of functions that does specify that +0.0 > -0.0 and any comparison between a number and NaN returns the number. \$\endgroup\$
    – Davislor
    Commented May 28 at 12:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.