1

I am getting the following warning for the below expression:

warning: suggest parentheses around assignment used as truth value [-Wparentheses]

while (*(arg_to++) = *(arg_from++));

What more brackets does it want from me?

Aquarius_Girl
  • 21,790
  • 65
  • 230
  • 411
  • 9
    Most compilers will warn about assignments as the top-level operation inside a `while` or `if` control expression. To the compiler it looks like you're writing `while (x = y)` and it thinks maybe you meant `while (x == y)`. To tell it you really want `while (x = y)`, you can add extra parentheses `while ((x = y))`, or to make it even more explicit, `while ((x = y) != 0)`. – Raymond Chen Jan 19 '18 at 04:51
  • 1
    I disagree with this warning message. I would suggest using `for` statement and loop body instead. –  Jan 19 '18 at 05:24
  • The warning message is asinine. Merely camouflaging the assignment with redundant parentheses is not an improvement. – user207421 Jan 19 '18 at 05:30
  • Also, the parenthesis around the increment expressions do nothing, the postfix increment operator already has higher precedence than dereference (prefix increment is equal precedence so there associativity rules). – SoronelHaetir Jan 19 '18 at 05:40
  • Exactly. There are already too many parentheses here. One pair is sufficient. @SoronelHaetir – user207421 Jan 19 '18 at 05:41
  • 3
    Possible duplicate of [Compiler warning - suggest parentheses around assignment used as truth value](https://stackoverflow.com/questions/5476759/compiler-warning-suggest-parentheses-around-assignment-used-as-truth-value) – Raymond Chen Jan 19 '18 at 19:16
  • 1
    @RaymondChen Why not dup in the other direction? [This answer](https://stackoverflow.com/a/48336995/712526) is probably the best of all, with a short answer at the top, and more detail below. – jpaugh Jan 19 '18 at 22:58

2 Answers2

2

The comment by @RaymondChen explains the warning message.

I suggest a little less cryptic and a bit verbose code.

while ( *arg_from != '\0' )
{
   *(arg_to++) = *(arg_from++);
}
*arg_to = '\0';
R Sahu
  • 204,454
  • 14
  • 159
  • 270
1

A historical reason that compilers give such a warning is that assignments within a loop condition

while (x = y)

are, more often than not, a typo in which the programmer actually intended a comparison.

while (x == y)

However, this is actually a valid expression, even if often a mistake, so compilers were (able to be) configured to warn about it (e.g. pointing out the use of = rather than ==). A number of different C and C++ compilers (when configured to higher warning levels) have given warnings to this effect for quite some time (I remember seeing it in C and C++ compilers from the mid 90s, and quite possibly it was in even older compilers).

However, one hacky technique used by programmers - when an assignment is actually intended and a compiler is warning about it - is to wrap the offending expression in brackets and do a comparison, such as

while ((x = y) != 0)

In response to use of such techniques, compilers were then configurable to give a more "informative" warning, such as suggesting adding brackets into while (x = y) for the programmer to consider. Again, compilers have given this warning for for some time (I can't recall, offhand, how many years ago compilers started doing that).

The real solution, however, is not to use hacky solutions such as adding brackets and comparisons to such expressions. A better solution is often to keep expressions as simple as possible - for example, to avoid having too many variables being modified within one expression.

An expression like

while (*(arg_to++) = *(arg_from++));

or (more minimalistically, removing unneeded brackets)

while (*arg_to++ = *arg_from++);

Has multiple effects - *arg_to is modified, and both arg_to and arg_from are incremented. To avoid multiple effects and side-effects in one expression, one can do

while (*arg_from)    // equivalent to *arg_from != '\0'
{
     *arg_to = *arg_from;
     ++arg_to;
     ++arg_from;
}

in which each expression modifies only one value. If you are willing to allow a statement or expression to modify no more than two values, this can be changed to

while (*arg_from)
{
     *arg_to++ = *arg_from++;
}

Both of these forms will tend to avoid warnings about suspicious assignments or hints to add brackets.

It is a bit subjective which of these forms is more "readable" - some people will prefer one, some the other. Generally speaking, however, expressions (whether in loop conditions or simple statements) that modify several values are more cryptic to mere mortals, so harder to get right. As well as being more likely to trigger compiler warnings.

Peter
  • 35,646
  • 4
  • 32
  • 74