openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.97k stars 2.56k forks source link

Reduce warnings count #1466

Closed bilderbuchi closed 10 years ago

bilderbuchi commented 12 years ago

To improve new processes like the deprecation warnings and the automated build server, it is imho important to reduce the number of warnings we get when compiling OF. Additional benefit: new users don't get scared by them. Since it's not always clear how to resolve certain warnings (e.g. is an unused variable a programming mistake or some leftover code cruft?), I intend this issue to be a discussion ground for advice on these matters. PRs are welcome (thanks @bostonbusmap for the most recent bout of fixes btw!)!

Updates: Linux OF compiles with 0 warnings. Other OSes, examples still TODO. MacOS has 24 warnings from the linker.

bilderbuchi commented 12 years ago

First question: in ofMesh, can I rely on index>=0 or can this be <0 for error conditions or so? There's several unsigned int comparisons like

void ofMesh::removeColor(int index){
  if(index >= vertices.size()){
    ofLog(OF_LOG_ERROR,"Trying to remove color out of range of this mesh. Taking no action.");

and I'm wondering if I can cast index to unsigned int or if I should better cast .size() to int instead?

bilderbuchi commented 12 years ago

Question 2: ‘bool ofCheckGLTypesEqual(int, int)’ defined but not used [-Wunused-function] ofTexture.cpp line 187 is here. Should I remove the function or add a declaration in ofTexture.h which seems to be missing?

damian0815 commented 12 years ago

re warnings, @ofTheo @ofZach what do you think about turning some of the warnings back on for Xcode projects? i'd be very keen to be warned when i forgot to add a return value, for one thing (-Wreturn-type)

bilderbuchi commented 12 years ago

It probably even makes sense to switch most (if not all) of them back on again. I just spent an hour or so and halved the amount of warnings I get, so I don't think it's too much effort to fix most of them.

Added benefits:

ofTheo commented 12 years ago

@damiannz sounds good to me! I think they got turned down a lot a while ago when there were tons of warnings from some libraries that we didn't have control over.

all for them if they are informative!

bilderbuchi commented 12 years ago

@ofTheo do you have any input regarding my two questions (comment1 and 2)?

ofTheo commented 12 years ago

@bilderbuchi

I think ofMesh::removeColor(int index) could be ofMesh::removeColor(unsigned int index)

regarding Q2: I think that is fine to leave it for now. it might get used - maybe @arturoc has some input on that one.

arturoc commented 12 years ago

yes, actually better to change it to ofIndexType which changes across platforms

bilderbuchi commented 12 years ago

Thank you, will do. Re: static bool ofCheckGLTypesEqual(int, int). that's a static function, so it's only visible in its own file, where it doesn't get used. It doesn't make sense to add a declaration for a static function, right? Why is this function static, anyway? @arturoc any insight what the best way to resolve this is? For reference, we're talking about this line throwing a warning cause the static function isn't getting used [Wunused-function]. Should we delete it?

arturoc commented 12 years ago

that's a function to check that the type and the internalType of a texture are compatible, i started it but never used it in the end. i left it there in purpose as a reference in case i wanted to implement it later but it's ok to remove it

bilderbuchi commented 12 years ago

great, will do. Rest of the work is done, only 3 warnings left(!), one I don't know how to fix ("dereferencing type-punned pointer will break strict-aliasing rules", ofUtils.cpp L347 and L446) and one I don't understand, could be a false positive ("‘image’ may be used uninitialised in this function [-Wuninitialized] ofCairoRenderer.cpp L499, at cairo_surface_destroy (image);) Thanks guys!

bilderbuchi commented 12 years ago

OK, Linux OF compiles with 0 warnings now. What about the other platforms? What's the approx. count there?

bilderbuchi commented 12 years ago

Also, fixing examples' warnings are still TODO.

kylemcdonald commented 12 years ago

for me, on osx 10.6 and xcode 3.2 building the emptyExample from scratch i see 24 warnings. they're all from the linker.

ld: warning: __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<unsigned int const, int> > >::allocate(unsigned long, void const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(Exif.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofTexture.o)
ld: warning: std::_Rb_tree<unsigned int, std::pair<unsigned int const, int>, std::_Select1st<std::pair<unsigned int const, int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, int> > >::_M_erase(std::_Rb_tree_node<std::pair<unsigned int const, int> >*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(Exif.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofTexture.o)
ld: warning: std::_Rb_tree<unsigned int, std::pair<unsigned int const, int>, std::_Select1st<std::pair<unsigned int const, int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, int> > >::_M_create_node(std::pair<unsigned int const, int> const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(Exif.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofTexture.o)
ld: warning: std::_Rb_tree<unsigned int, std::pair<unsigned int const, int>, std::_Select1st<std::pair<unsigned int const, int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, int> > >::_M_insert(std::_Rb_tree_node_base*, std::_Rb_tree_node_base*, std::pair<unsigned int const, int> const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(Exif.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofTexture.o)
ld: warning: std::map<unsigned int, int, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, int> > >::operator[](unsigned int const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(Exif.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofTexture.o)
ld: warning: __gnu_cxx::new_allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::allocate(unsigned long, void const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(IPTC.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::_Vector_base<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::~_Vector_base()has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(IPTC.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::~vector()has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(IPTC.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::_M_insert_aux(__gnu_cxx::__normal_iterator<std::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(IPTC.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::push_back(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(IPTC.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::basic_string<char, std::char_traits<char>, std::allocator<char> > std::operator+<char, std::char_traits<char>, std::allocator<char> >(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(PluginEXR.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofGLRenderer.o)
ld: warning: std::basic_string<char, std::char_traits<char>, std::allocator<char> > std::operator+<char, std::char_traits<char>, std::allocator<char> >(char const*, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(PluginEXR.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofGLRenderer.o)
ld: warning: __gnu_cxx::new_allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::allocate(unsigned long, void const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfHeader.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::_Vector_base<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::~_Vector_base()has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfHeader.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::~vector()has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfHeader.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: __gnu_cxx::new_allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::allocate(unsigned long, void const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStringVectorAttribute.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::_M_insert_aux(__gnu_cxx::__normal_iterator<std::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStringVectorAttribute.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::push_back(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStringVectorAttribute.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::basic_string<char, std::char_traits<char>, std::allocator<char> > std::operator+<char, std::char_traits<char>, std::allocator<char> >(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfRgbaFile.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofGLRenderer.o)
ld: warning: __gnu_cxx::new_allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::allocate(unsigned long, void const*)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStandardAttributes.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::_Vector_base<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::~_Vector_base()has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStandardAttributes.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::_Vector_base<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::_Vector_base(unsigned long, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStandardAttributes.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::~vector()has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStandardAttributes.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
ld: warning: std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::vector(std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&)has different visibility (hidden) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ImfStandardAttributes.o-i386) and (default) in /Users/kyle/Documents/openFrameworks/libs/openFrameworksCompiled/lib/osx/openFrameworksDebug.a(ofAppGlutWindow.o)
bilderbuchi commented 12 years ago

http://stackoverflow.com/questions/4352920/warning-xxxx-has-different-visibility-default-in-yyyy-and-hidden-in-zzzz is probably a way to resolving these warnings.

ofTheo commented 12 years ago

@kylemcdonald yeah - actually that was one of the reasons warnings were disabled on xcode. I think it basically requires rebuilding all osx libs with the same settings.

Its a pain but we also need to get things standerdized as we'll need to build 64bit libs soon also. I should add that to my assigned list.

bilderbuchi commented 12 years ago

@ofTheo: I think it's not that bad - probably we only have to change the setting mentioned in the SO answer in some parts of the OF project. If you look at the warning log @kylemcdonald posted, you'll see that the visibility clashes are quite few, and paired like this:

hidden, default:  
Exif.o-i386, ofTexture.o
ImfRgbaFile.o-i386, ofGLRenderer.o
PluginEXR.o-i386, ofGLRenderer.o
IPTC.o-i386, ofAppGlutWindow.o
ImfStringVectorAttribute.o-i386, ofAppGlutWindow.o
ImfHeader.o-i386, ofAppGlutWindow.o
ImfStandardAttributes.o-i386, ofAppGlutWindow.o

I guess the "hidden" ones are system libs or somesuch, and we may only have to adjust the compilation settings for ofTexture, ofGLRenderer and ofAppGlutWindow. So maybe we get away with only an OF lib recompilation. Take that witha grain of salt, though, due to my non-experience with XCode.

underdoeg commented 12 years ago

@ofTheo What kind of warnings were disabled? This might be an issue. I recently worked on ofxTimeline for linux and it had a function implementation like this:

bool function(){ return; }

This compiled in XCod (with a warning I think) but when I tried in linux I got an error (which is what is supposed to happen, IMO) I think we should try to have about the same compile settings on each platform so you won't be faced with some unexpected errors when you port it.

ofTheo commented 12 years ago

@bilderbuchi that sounds promising! I'll see if that works.

@underdoeg I think we had all warnings disabled at one point. Not sure if that would have caught that bug as that feels like it should error on xcode too. Agreed that we should try and get things closer, though with apple using LLVM now and linux GCC I think we will always have some differences between the two.

bilderbuchi commented 11 years ago

PR #1838 contributes to solving this issue.

pizthewiz commented 10 years ago

Is this still an issue or can each error/warning be treated as exceptional?

bilderbuchi commented 10 years ago

The situation has become much better. I'm not sure if the warnings kyle mentioned above still occur (can you test?). If so, we should probably take this into account somehow when building new libs with apothecary. If not, we can close this. Reducing warnings count is an ongoing thing, we'll just open another issue.

admsyn commented 10 years ago

The warnings kyle mentioned don't occur anymore. Building a vanilla project on Xcode right now nets only 3 warnings (2 nontrivial ones for using QTKit, another for "update to recommended settings").

:+1: for closing this, QTKit-use is already being addressed for 0.9.0

underdoeg commented 10 years ago

on linux I only get warnings about an extra ; here:

https://github.com/openframeworks/openFrameworks/blob/9d06863b11a2a6be131d9c42152f4d0bcc9653bc/libs/openFrameworks/types/ofParameter.h#L571 https://github.com/openframeworks/openFrameworks/blob/9d06863b11a2a6be131d9c42152f4d0bcc9653bc/libs/openFrameworks/types/ofParameter.h#L579 https://github.com/openframeworks/openFrameworks/blob/9d06863b11a2a6be131d9c42152f4d0bcc9653bc/libs/openFrameworks/types/ofParameter.h#L583

bilderbuchi commented 10 years ago

ha, good point. could you PR a fix?

underdoeg commented 10 years ago

will do

bilderbuchi commented 10 years ago

thanks!

underdoeg commented 10 years ago

here we go https://github.com/openframeworks/openFrameworks/pull/3161

This only leaves one warning on my system.