stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r409484 - in /incubator/stdcxx/trunk/tests: include/21.strings.h src/21.strings.cpp
Date Fri, 26 May 2006 17:21:39 GMT
Anton Pevtsov wrote:
> Martin, the test driver build on msvc-7.1 produces warnings. 
> 
> 21.strings.cpp
> D:\Projects\RogueWave\work\stdlib\tests\src\../include\21.strings.h(108)
> : warning C4554: '<<' : check operator precedence for possible error;
> use parentheses to clarify precedence
> 
> The cause is 
> 	1 << fid_bits + 6 * arg_bits
> 
> The possible fix is
> 	1 << (fid_bits + 6 * arg_bits)
> 
> This warning looks bogus, but I think that parentheses really clarify
> the statement.

How annoying! I guess the authors of the warning must have a problem
remembering the precedence of operator<< and assume that the rest of
us do too. Odd that operators + and * don't seem to give them trouble.
Your fix looks good.

> 
> 
> And (in several places):
> 
> 21.strings.cpp
> \Projects\RogueWave\work\stdlib\tests\src\21.strings.cpp(190) : warning
> C4800: 'int' : forcing value to bool 'true' or 'false' (performance
> warning)

Sigh, yet another braindead warning(*).

> 
> The cause is:
> 	const bool is_member = Ids::bit_member & which;
> 
> The possible fix is
> 	const bool is_member = (Ids::bit_member & which) > 0;

Let's go with this instead (the result could be negative if
bit_member is the sign bit):

     const bool is_member = 0 != (Ids::bit_member & which);

Thanks
Martin

(*) This one is especially silly. Consider the three equivalent
functions in the program below. The result of the & expressions
is in all cases an int yet the warning is issued only in 2 out
of the 6 cases. Comparing the disassembly of foo and bar shows
that the code in both cases in nearly identical with the only
difference being the use of the 8-bit al register in foo and
the 32-bit eax register in bar, certainly not something worth
warning about (but even if it was MSVC should be able to generate
the same code in both cases just like gcc and Intel C++ are).

$ cat -n t.cpp && cl -Fa -O2 -W3 -nologo t.cpp
      1  bool foo (int i)
      2  {
      3      if (i & 1) return true;   // no warning
      4      if (i & 2) return true;   // no warning
      5      return false;
      6  }
      7
      8  bool bar (int i)
      9  {
     10      const bool b1 = i & 1;   // no warning
     11      if (b1) return true;
     12      const bool b2 = i & 2;   // warning
     13      if (b2) return true;
     14      return false;
     15  }
     16
     17  bool foobar (int i)
     18  {
     19      enum { e1 = 1, e2 = 2 };
     20      const bool b1 = i & e1;   // no warning
     21      if (b1) return true;
     22      const bool b2 = i & e2;   // warning
     23      if (b2) return true;
     24      return false;
     25  }
     26
     27
t.cpp
t.cpp(12) : warning C4800: 'int' : forcing value to bool 'true' or 
'false' (performance warning)
t.cpp(22) : warning C4800: 'int' : forcing value to bool 'true' or 
'false' (performance warning)

Here's the disassemly for foo:

?foo@@YA_NH@Z PROC                                      ; foo, COMDAT
         mov     al, BYTE PTR _i$[esp-4]
         test    al, 1
         je      SHORT $LN2@foo
         mov     al, 1
         ret     0
         shr     al, 1
         and     al, 1
         ret     0

And here's bar (foobar looks the same):

?bar@@YA_NH@Z PROC                                      ; bar, COMDAT
         mov     eax, DWORD PTR _i$[esp-4]
         test    al, 1
         je      SHORT $LN2@bar
         mov     al, 1
         ret     0
         shr     eax, 1
         and     al, 1
         ret     0

Mime
View raw message