[omniORB] omniORB 4.2 : Visual C++ Compiler Warning L4 C4706 "assignment within conditional expression"

[Duncan Grisby]

On Wed, 2018-10-10 at 09:46 +0200, Martin Ba via omniORB-list wrote:

> OmniORB's Source code makes use of the following construct:
> 
>      if ((pd_data = v.pd_data)) ...

[...]
> Even though it already uses double-parens, VC++ isn't satisfied and
> will issue a [C4706 assignment within conditional expression][1] for
> this line.
> 
> C4706 is a Level 4 warning, but we selectively enable it because we 
> found it quite useful in other contexts.
> 
> Apparently, MS thinks that double-parens are not enough, because
> they explicity call this out in their [docs][1]:
> 
>    > The warning will occur even if you
>    > double the parentheses around the test condition

How helpful of Microsoft to choose to wilfully ignore the convention
used in other compilers...

> The warning can be fixed by changing the line to:
> 
>      if ((pd_data = v.pd_data) != 0) ...

I have to say, I think that makes the already-confusing code even more
confusing. I think it would be better just to expand the code to avoid
the assignment inside the conditional altogether, so it becomes like
this:

    inline Servant_var& operator= (const Servant_var& v) {
      if (v.pd_data != pd_data) {
	if (pd_data)
          pd_data->_remove_ref();

        pd_data = v.pd_data;
        if (pd_data)
          pd_data->_add_ref();
      }
      return *this;
    }

I've checked that in, plus a similar change in
CORBA_ValueBase_vartypes.h that had the same code pattern.

Duncan.

-- 
 -- Duncan Grisby         --
  -- duncan at grisby.org     --
   -- http://www.grisby.org --