Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix warnings, add make/cmake STOP_ON_WARNING flag #164

Merged
merged 4 commits into from Oct 16, 2012

Conversation

Projects
None yet
3 participants
Owner

lgritz commented Oct 5, 2012

Fix some compiler warnings related to int vs unsigned.

Also add make/cmake flags so you can

make STOP_ON_WARNING=0

If you are on a new platform and finding compiler warnings, this makes it easier to get "unstuck" by forcing the build to continue rather than stop when compiler warnings are encountered. The default is still to treat warnings as if they are errors.

Owner

lgritz commented Oct 16, 2012

Any objections?

@aconty aconty and 1 other commented on an outdated diff Oct 16, 2012

src/CMakeLists.txt
if (NOT MSVC)
- add_definitions ("-Wall -Werror")
+ add_definitions ("-Wall")
+ if (STOP_ON_WARNING)
+ add_definitions ("-Wall -Werror")
@aconty

aconty Oct 16, 2012

Member

I guess you just want to add -Werror? the other one is already there, right?

@lgritz

lgritz Oct 16, 2012

Owner

Yeah, I'll remove the extra -Wall.

Member

aconty commented Oct 16, 2012

LGTM

@fpsunflower fpsunflower and 1 other commented on an outdated diff Oct 16, 2012

src/liboslexec/gabornoise.cpp
quick_floor(p[2]), seed };
- m_seed = inthash<4>(pi);
+ m_seed = inthash<4>((const unsigned int *)pi);
@fpsunflower

fpsunflower Oct 16, 2012

Member

I don't understand this change. It was declared as unsigned int, and you changed it to int and cast it back to unsigned int ?

@fpsunflower

fpsunflower Oct 16, 2012

Member

If the problem is that quick_floor() is returning int, it might be clearer to just add unsigned() around each entry.

@lgritz

lgritz Oct 16, 2012

Owner

quick_floor returns int, but inthash<> wants unsigned values (it only cares about the bits). It would be equivalent to leave pi[] unsigned but then cast/convert each of the four values put into it. The way I did it -- make pi[] the right type to hold the quick_floor results, but cast the pointer for the hash -- required only one cast. Do you think the other way is much more clear?

@fpsunflower

fpsunflower Oct 16, 2012

Member

I think its better since it avoids a pointer cast (which gcc can get picky about).

@lgritz

lgritz Oct 16, 2012

Owner

OK, how about now?

Member

fpsunflower commented Oct 16, 2012

LGTM

@lgritz lgritz added a commit that referenced this pull request Oct 16, 2012

@lgritz lgritz Merge pull request #164 from lgritz/lg-warnings
Fix compiler warnings, add make/cmake STOP_ON_WARNING flag
b6e27d2

@lgritz lgritz merged commit b6e27d2 into imageworks:master Oct 16, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment