h3rb / openshadinglanguage

Automatically exported from code.google.com/p/openshadinglanguage
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

other compiler warnings that kill the compile #55

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.make osl

osl/src/liboslexec/dual.h: 430
 warning: suggest explicit braces to avoid ambiguous ‘else’

osl/src/liboslcomp/oslcomp.cpp:1035: warning: suggest parentheses around &&
within ||

the latter one looks like a bug to me. && has precedence over ||, so 

if ( A && 
    B || C )  // as it is written  

evals as  (A && B) || C 

right?  So C by itself can make the evaluation true all by itself.
But the logic as I read it and the indentation seem to suggest A 
needs to be true and then either B or C.

Original issue reported on code.google.com by crunch...@gmail.com on 17 Feb 2010 at 4:37

GoogleCodeExporter commented 9 years ago
I missed a couple of others:

osl/src/liboslexec/bsdf_cloth.cpp:88: warning: ‘typedef’ was ignored in 
this declaration

the code:

typedef struct threadSegment
{
(stuff)
} /* something missing here */ ;

also,  compiling osl/src/oslc/oslcmain.cpp
 I don't see EXIT_FAILURE or EXIT_SUCCESS defined anywhere.  

Original comment by crunch...@gmail.com on 17 Feb 2010 at 4:54

GoogleCodeExporter commented 9 years ago
Good catches!  I will fix these and submit a review right away.

Original comment by larrygr...@gmail.com on 17 Feb 2010 at 6:26

GoogleCodeExporter commented 9 years ago
cool, but it makes me wonder about how you get it to compile at all, as 
documented in
issue 54.  Rene suggested I was either "just doing it (typing 'make') wrong" or
perhaps a problem with my OS distribution, but undefined constants among other 
things
are not something I can screw up. 

And perhaps coincidentally, or not, I get the following results from running 
testcases.

84% tests passed, 8 tests failed out of 51

The following tests FAILED:
      9 - derivs (Failed)
     26 - matrix (Failed)
     27 - message (Failed)
     30 - noise (Failed)
     31 - pnoise (Failed)
     34 - string (Failed)
     46 - transform (Failed)
     49 - vecctr (Failed)

I decided to declare victory for the day after getting everything to compile and
actually run the test suite, and have not looked into the test failures. 

Original comment by crunch...@gmail.com on 17 Feb 2010 at 8:17

GoogleCodeExporter commented 9 years ago
These test failures are just issues of LSB's on different platforms.  We have 
them on
Linux as well.  (Our checked-in reference output is on Mac.)  This is 
definitely a
problem we need to address, but these "failures" are not actually a problem.

I suspect that your problems compiling are a result of using a different 
version of
gcc than we are using.  Probably a more modern version is warning about things 
that
the older versions do not.  The "undefined constant" is something defined in
stdlib.h, which was not included directly (though it is now in my review), so I 
guess
in older gcc it was indirectly included by other header files, and now no 
longer is.
 All fairly minor issues.

Original comment by larrygr...@gmail.com on 17 Feb 2010 at 9:27

GoogleCodeExporter commented 9 years ago
I am using 

gcc version 4.3.2 20081105 (Red Hat 4.3.2-7)   Target: x86_64-redhat-linux

I am relieved to hear that failures on linux are not my fault.  Rene told me 
that all
the tests should pass.  My self esteem has taken a beating. 

I checked out the 'review' a couple of hours ago. That's pretty cool, especially
where you can insert comments into proposed code.

I actually had one more compile error that made no sense to me, so I didn't 
include
it.  I was seeing a complaint that strcmp was undefined even though there was a 

#include <string>

I won't mention it formally unless I can figure out why it's happening. I added 
the
old-style 

#include "string.h" 

and the compiler was happy. 

Original comment by crunch...@gmail.com on 17 Feb 2010 at 9:51

GoogleCodeExporter commented 9 years ago
#include <string> is the C++ header that defines std::string

#include <cstring> is the C++ way to get the C string header file

#include <string.h> is the C way to get the C string header file

Quotes should be reserved for project headers, never system ones.

In gcc 4.3+, lots of these headers were cleaned up. For example, <string> was
including <cstring>, but it no longer does (because it shouldn't). Therefore 
code
that was assuming that <cstring> was getting included automatically will fail to
compile (as ours did).

Original comment by cku...@gmail.com on 17 Feb 2010 at 10:15