2

I have a struct of the form:

struct foo {
   bool has_value;
   int value;
}

I want to create a macro, which can be used to assign value as follows:

ASSIGN(foo) = 0;
   or
ASSIGN(foo) += 5;

when the macro is:

#define ASSIGN(s) \
    s->has_value = true; s->value

However, the macro is not if safe. In the example below if will "eat" the first part of the ASSIGN macro.

if (cond)
    ASSIGN(foo) += 5;

Any proposal on how to make ASSIGN if safe?

Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128
dimba
  • 26,717
  • 34
  • 141
  • 196
  • 10
    May I state the canonical "do not use macro" here? – Eugene Sh. Jan 27 '20 at 21:20
  • 2
    When doing `ASSIGN(foo) += 5;`, will `value` always be initialized beforehand? – HolyBlackCat Jan 27 '20 at 21:22
  • 4
    I suggest you define functions `assign_foo(struct foo*, int)` and `inc_foo(struct foo*, int)` – Barmar Jan 27 '20 at 21:25
  • 2
    If you find yourself trying to make C looks like a different language, you probably ought to be using the language you are trying to make it look like instead. A C++ class with overloaded assignment operators is what you want here perhaps? – Clifford Jan 27 '20 at 21:37

3 Answers3

4

Use a comma operator, and throw in parens for good measure.

#define ASSIGN(s) \
    ((s)->has_value = true), (s)->value

In an if-statement it would expand to:

if (cond)
    ((foo)->has_value = true), (foo)->value += 5;

However, macros in this usage are a very bad idea.
You can almost always come up with abusive code that will break your macro.

Even if it works, it will confuse future programmers who may read your code.


Here is one bad way this macro will break:
ASSIGN(foo) = M_PI;
printf("Sin of PI/2 = %f\n", sin(ASSIGN(foo) *= 0.5));

You might expect that to print 1.
It probably won't even compile.

abelenky
  • 63,815
  • 23
  • 109
  • 159
1

Any proposal on how to make ASSIGN if safe?

You cannot, in general, do this. You should not use macros for an lvalue. It's a terrible idea, and it will most probably lead to crazy and impossible to find bugs.


This really seems like an XY problem. In your case, as I understand, you want to create a macro to simplify an expression that should be done over and over multiple times in your code.

Instead of using a simple one-argument macro, you can define a macro with two arguments. This way, it will be more compatible with the rest of your code, while still achieving the same result, using a more consistent semantic, which is also "if-safe".

Here it is (I'm calling it ASSIGN_FOO just to distinguish it from yours):

#define ASSIGN_FOO(x, v) do { (x)->has_value = true; (x)->value = v; } while (0)

struct foo var;
ASSIGN_FOO(var, 123);

If you're wondering about that do { ... } while (0), have a look here.

In case you want the macro to return the assigned value though (like you would expect from a normal assignment), this is not a good option.

Instead of using a macro, you can define an inline function, declaring it with __attribute__ ((always_inline)). This way, the compiler integrates the function's code directly into the caller, and the function will act exactly as a macro when the program is compiled, except that it is now more powerful as it can be used in more contexts.

inline int __attribute__ ((always_inline)) assign_foo(struct foo *x, int value) {
    x->has_value = true;
    x->value = value;
    return x->value;
}

struct foo var;
assign_foo(var, 123);

In addition to this, it doesn't make much sense to use the macro you defined when updating the value in your struct, as it can easily lead to unwanted undefined behavior, like the following:

struct foo var;
ASSIGN(var) += 5;

Which expands to:

var->has_value = true; var->value += 5; // Undefined behavior, var->value used uninitialized!

The solution here is:

  1. If you already know that the value is present, it doesn't make sense to re-assign has_value = true, you can just do the increment directly:

    var->value += 10;
    
  2. If you don't know if the value is present, use a function to do it safely instead:

    inline int __attribute__ ((always_inline)) increment_foo(struct foo *x, int value) {
        if (!x->has_value) {
            x->has_value = true;
            x->value = value;
        } else {
            x->value += value;
        }
    
        return x->value;
    }
    
    increment_foo(var, 10);
    

Comparison:

struct foo x;
printf("%d\n", ASSIGN(x) = 3);    // Compilation error.
printf("%d\n", ASSIGN_FOO(x, 3);  // Compilation error.
printf("%d\n", assign_foo(x, 3)); // No problem.

struct foo y;
printf("%d\n", ASSIGN(y) += 3);           // Compilation error.
ASSIGN(y) += 3; printf("%d\n", y->value); // Undefined behavior.
printf("%d\n", increment_foo(y, 3));      // No problem.
Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128
0

Such syntax-breaking macros are usually a bad idea but you can do it with

#define ASSIGN(s) ((s).has_value=1,&(s))->value

if you want it to take an lvalue or

#define ASSIGN(sp) ((sp)->has_value=1,(sp))->value

if you want it to take a pointer.

Working example (https://godbolt.org/z/fHAGRa):

#include <stdbool.h>
#include <stdio.h>
struct foo {
   bool has_value;
   int value;
};

#define ASSIGN(s) ((s).has_value=1,&(s))->value

int main()
{
    struct foo x;
    ASSIGN(x) = 21;
    printf("%d\n", ASSIGN(x) *= 2 ); //prints 42
    return ASSIGN(x); //returns the last value (42)
}

It doesn't protect from double-evaluation—for that you'd need the expression statement extension or a helper inline function such as

static inline struct foo *set_has_value_return_ptr(struct foo *X){ 
      return X->has_value=1, X; 
}
#define ASSIGN(s) (set_has_value_return_ptr(&(s)))->value

Explanation:

The naive ((s).has_value=1,(s).value) or ((s).has_value=1,(s)).value wouldn't work because of lvalue to rvalue conversion done by the comma operator, but ((s).has_value=1,&(s))->value or (*((s).has_value=1,&(s))).value will because pointer dereferencing (*) and -> always yield lvalues regardless of whether their pointer argument was an lvalue or an rvalue.

Petr Skocik
  • 58,047
  • 6
  • 95
  • 142