search

betajaen / gorilla

Minimal HUD and Overlay replacement system for Ogre 1.x only
60 stars 11 forks source link

MinGW Complains About Unknown Pragma #22

Open k6l2 opened 9 years ago

k6l2 commented 9 years ago

My only complaint so far.

Get rid of this:

pragma warning ( disable : 4244 )

I don't know what crappy compiler you're using _cough_VScough, but why would I even want to disable warnings anyway? At least make it so this pragma is only active on that crappy compiler.

Great job otherwise, keep up the good work!

ViteFalcon commented 9 years ago

You may want to cough alteast 4 more times to cover 2 more compilers (GCC[1] and Clang[2]), since they all support disabling warnings and have issues of their own that may be crappy for others.

Joke aside, AFAIK warnings were disabled mainly due to third party libs like Boost that get implicitly or explicitly included and have type 'mismatches' that the VS compilers warn about. Like int variable being assigned from size_t variable [3]. A simple case of code in point int strLength = myStdString.length() or a for-loop that uses an int variable to iterate but compares termination condition against a size_t value.

You said, "At least make it so this pragma is only active on that crappy compiler.". Well, so could you. How about submitting a patch?

[1] http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html [2] http://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas [3] http://msdn.microsoft.com/en-us/library/th7a07tz.aspx

betajaen commented 9 years ago

I agree, if your concerned about it so much - then do a patch, or better still - find and fix the code that causes the warning, then we can remove it completely.

k6l2 commented 9 years ago

Hohoho, touché. I am glad I at least sparked some conversation and an educational compiler post. I use MinGW myself as of late, and honestly it was a very minor annoyance nothing more. Once that pragma is commented out I get no further disturbances from the command line. Once again, very much enjoying my experience with Gorilla so far. Also on an unrelated note, I'm just getting over a cold that's lasted since late October, so I think I've coughed enough in recent days.. ;)

betajaen commented 9 years ago

I think both ViteFalcon and I aren't commenting about we should have a patch to change the warning per compiler, but solve the issue itself why the warning is there in the first place. If we change the code that the warning has an issue about, then it's no longer needed, and solves all the problems with all the compilers.

In short, treat the disease not the symptoms. :)

ViteFalcon commented 9 years ago

I concur.