Re: [ft-devel] MSVC build woes !

2013-06-14 Thread John Emmas

On 12/06/2013 22:34, Werner LEMBERG wrote:
Please test the current git for the next iteration... Werner 


Great stuff, Werner - no warnings at all !!

Thanks for all your perseverance,

John

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-12 Thread Werner LEMBERG

 Okay, here's my original Buildlog output text (in HTML format).
 I've packed it into a zip file this time because the first one was
 too big.

Thanks again!  I've again disabled MSVC warning C4127 in the git
repository; especially after looking at

  https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines

I now believe that the original solution of disabling this warning
completely is the right thing.

Theoretically, there shouldn't be any warnings now in an MSVC build.


   Werner

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-12 Thread John Emmas

On 12/06/2013 10:10, Werner LEMBERG wrote:


I've again disabled MSVC warning C4127 in the git
repository; especially after looking at

   https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines

I now believe that the original solution of disabling this warning
completely is the right thing.

Theoretically, there shouldn't be any warnings now in an MSVC build.



Thanks for all your hard work Werner and for everybody's input on this.  
I only spotted one problem


In 'ftserv.h' you've implemented the pragma like this:-

#if defined( _MSC_VER )  /* Visual C++ (and Intel C++) */
  /* We disable the warning `conditional expression is */
  /* constant' in order to compile cleanly with the maximum */
  /* level of warnings. */
#pragma warning( push )
#pragma warning( disable : 4127 )
#endif /* _MSC_VER */

// [ ... ] rest of header file

#if defined( _MSC_VER )
#pragma warning( pop )
#endif

But of course that doesn't work - because the 'pop' will re-enable the 
warning as soon as that header file finishes processing.  By the time 
the compiler wends its way back to the original source file, warning 
4127 is enabled again.  The way you did it in 'ftdebug.h' works fine though.


John

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


[ft-devel] MSVC build woes !

2013-06-11 Thread John Emmas

Hi guys,

I noticed that these lines have recently been removed from 'ftdebug.h'

#if defined( _MSC_VER )  /* Visual C++ (and Intel C++) */
  /* We disable the warning `conditional expression is 
constant' here */
  /* in order to compile cleanly with the maximum level of 
warnings.  */

#pragma warning( disable : 4127 )
l#endif /* _MSC_VER */

I think the comment probably meant minimum level of warnings but no 
matter  Previously (about 5 weeks ago) those lines were at about 
line 251.  Unfortunately, removing them now produces hundreds of 
unnecessary warnings when building with MSVC.  If it isn't possible to 
restore them I can suggest an alternative solution but my guess is that 
they've just gotten removed accidentally.  Any chance of putting them back?


John

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-11 Thread Werner LEMBERG

 I noticed that these lines have recently been removed from
 'ftdebug.h' [...]

Please send me all the warnings.  The removal is of temporary nature
only: I'm going to re-add this pragma as soon as I know what it
exactly suppresses (I don't have MSVC).


Werner

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-11 Thread John Emmas

On 11/06/2013 11:56, Werner LEMBERG wrote:

Please send me all the warnings.  The removal is of temporary nature
only: I'm going to re-add this pragma as soon as I know what it
exactly suppresses (I don't have MSVC).



Hi Werner, I understand.  Here's a good example to get you started. I 
can find you the other instances, if necessary...


The warnings are typically caused by FT_ASSERT and take the following form:-

   : warning C4127: conditional expression is constant

FT_ASSERT is defined in 'ftdebug.h' and looks like this:-

#define FT_ASSERT( condition 
)  \

do\
{ \
if ( !( condition ) 
)   \
  FT_Panic( assertion failed on line %d of file 
%s\n, \
__LINE__, __FILE__ 
);   \

  } while ( 0 )

It usually gets called something like this:-

  FT_ASSERT( some_var == some_other_var );

and wherever it gets called, I get warning 4127.  MSVC is warning that 
there's no point putting this in a do/while loop, since nothing ever 
changes.  Rather than re-instate the pragma, a better solution would be 
to remove 'do' and 'while'.  At first glance, it doesn't look like 
there'd be any adverse effects.  If you agree that removing 'do' and 
'while' is the way forward, I'll find you the other occurrences.  Regards,


John

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-11 Thread Werner LEMBERG

 Please send me all the warnings.  The removal is of temporary
 nature only: I'm going to re-add this pragma as soon as I know what
 it exactly suppresses (I don't have MSVC).
 
 Hi Werner, I understand.  Here's a good example to get you started.
 [...]

Thanks for the explanations!  What I really want to do is to limit the
pragma (if we can't get rid of it, as you suggest) with

   #pragma warning(push)
   ...
   #pragma warning(pop)

so that only the offending code gets controlled by the pragma instead
of all compilation units.  So please send me the complete log of a
FreeType compilation (compressed if necessary).


Werner

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-11 Thread Robin Watts

On 11/06/2013 13:08, John Emmas wrote:

#define FT_ASSERT( condition  )   \
do\
{ \
if ( !( condition ) ) \
FT_Panic( assertion failed on line %d of file %s\n, \
  __LINE__, __FILE__ );   \
} while ( 0 )

It usually gets called something like this:-

   FT_ASSERT( some_var == some_other_var );

and wherever it gets called, I get warning 4127.


Indeed.


MSVC is warning that  there's no point putting this in a do/while loop,
since nothing ever changes.  Rather than re-instate the pragma, a
better solution would be to remove 'do' and 'while'.


No, no, no! Absolutely wrong!

There is a VERY good reason for the while being there. I will try to 
explain:


Imagine that you have the following code:

  if (x == 0)
  FT_ASSERT(some_condition);
  else
  return 42;
  return 23;

Without the while loop, the code would expand to:

   if (x == 0)
  if (!some_condition)
  FT_Panic( ... );
  else
  return 42;
   return 23;

(indentation changed for clarity)

i.e. the behaviour of the code is changed for x == 0; in the first case, 
the assert would be tested, then 23 would be returned. In the second the 
assert would be tested, then 42 would be returned.


By putting the do/while loop in, you avoid the possibility of the macro 
changing the surrounding control flow.


When writing macros in C it is considered good style to try to minimise 
unintended consequences of the macros use (a good example of The 
Principle of Least Surprise).



At first glance, it doesn't look like
there'd be any adverse effects.  If you agree that removing 'do' and
'while' is the way forward, I'll find you the other occurrences.  Regards,


Those examples of do and while are put there very deliberately. Removing 
them would be a harmful step.


It is true that the MSVC warning is unhelpful, so finding a way to stop 
it (such as by putting the original set of lines back in) would be 
preferable.


Robin


___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-11 Thread John Emmas

On 11/06/2013 15:14, Robin Watts wrote:


There is a VERY good reason for the while being there. I will try to 
explain:


Imagine that you have the following code:

  if (x == 0)
  FT_ASSERT(some_condition);
  else
  return 42;
  return 23;

Without the while loop, the code would expand to:

   if (x == 0)
  if (!some_condition)
  FT_Panic( ... );
  else
  return 42;
   return 23;

(indentation changed for clarity)

i.e. the behaviour of the code is changed for x == 0;



Thanks for the explanation Robin,

That being the case, this solution works for MSVC:-

#define FT_ASSERT( condition )  \
  {\
if ( !( condition ) ) 
 \
  FT_Panic( assertion failed on line %d of file 
%s\n, \
__LINE__, __FILE__ ); 
  \

  }

In fact, a similar strategy is already being used in the definition of  
'FT_THROW' so I guess it should work for other compilers too.

John

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] MSVC build woes !

2013-06-11 Thread J. Ali Harlow
On Tue, Jun 11, 2013, at 03:56 PM, John Emmas wrote:
 On 11/06/2013 15:14, Robin Watts wrote:
 
  There is a VERY good reason for the while being there. I will try to 
  explain:
 
  Imagine that you have the following code:
 
if (x == 0)
FT_ASSERT(some_condition);
else
return 42;
return 23;
 
  Without the while loop, the code would expand to:
 
 if (x == 0)
if (!some_condition)
FT_Panic( ... );
else
return 42;
 return 23;
 
  (indentation changed for clarity)
 
  i.e. the behaviour of the code is changed for x == 0;
 
 
 Thanks for the explanation Robin,
 
 That being the case, this solution works for MSVC:-
 
  #define FT_ASSERT( condition )  \
{\
  if ( !( condition ) ) 
   \
FT_Panic( assertion failed on line %d of file 
 %s\n, \
  __LINE__, __FILE__ ); 
\
}
 
 In fact, a similar strategy is already being used in the definition of  
 'FT_THROW' so I guess it should work for other compilers too.

That's not good either:

FT_ASSERT(some_condition);

results in two statements which means that Robin's example:

if (x == 0)
FT_ASSERT(some_condition);
else
return 42;
return 23;

won't work. Better is:

#define FT_ASSERT( condition )  \
  if ( !( condition ) ) 
   \
FT_Panic( assertion failed on line %d of file 
 %s\n, \
  __LINE__, __FILE__ ); 
else \

which at least results in correct code if FT_ASSERT is used correctly,
but can fail in unexpected ways if you forget to put the semi-colon
after the macro call. There really is a very good reason why do ...
while(0) is standard.

Ali.

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel