double Item_cache_decimal::val_real()
{
DBUG_ASSERT(fixed);
double res;
if (!value_cached && !cache_value())
return NULL;I filed bug 53445 for this. MySQL has been very good at fixing things lately. This should also get fixed. Visit the bug and subscribe to it or update it.
Searching for "site:bugs.mysql.com compiler warning" finds a lot of entries.
Posts
Not checking for errors/exceptions has been a cause of additional troubleshooting/investigation work soooo many times....
ReplyDeleteOf course, just asserting in various case is an over-reaction too ;-)
While I can agree with best coding practices, when building a stable build of source I do not like having my build logs loaded up with the messages generated by -Wall, and would not ever want my program to not compile at all due to an issue like this (Where it will function programatically but does not adhere to the C standard at this point in time.). I agree that the people working on the source tree should be compiling with those flags, but that is something that for someone like myself (more sysadmin than coder) would lead to more hassle then it is worth. My 2 coppers. Also reading bug 42733 only using -O2 or -O3 along with -Wall leads to that, and in this case this code was written prior to GNU C 3.xx, and without strict aliasing as an available feature. Can it be fixed? Absolutely. Is it the end of the world? Not even close. Just wait till the next version of the GNU C compiler and libs come out, it's always a nightmare for a half decade or so.
ReplyDeleteWhy would a build log fill up with warnings. Warnings should be fixed.
ReplyDeleteI hope you are building with -fno-strict-aliasing. Depending on the version of MySQL that you use, that option is required to avoid intermittent crashes.
This comment has been removed by the author.
ReplyDeleteIf you are using -Wall then you are getting warnings about it, if you are using -O2 or higher then you are indeed using strict aliasing.
ReplyDelete-Wall enables all warnings, including -Wstrict-aliasing. I am not sure, but I beleive if you use -fno-strick-aliasing and -Wall you will still get warnings about strict aliasing even though you are compiling without it.
ReplyDeleteFixing warnings and building with -Wall -Werror is an easy way to prevent simple coding mistakes, handle error conditions you may not have thought of, avoid logic errors, and generally write better code.
ReplyDeleteI don't advocate being overly pedantic, but compilers warnings are there for a good reason. Ignoring standards compliance may cause invalid code generation with future compilers. There's no reason to accumulate technical liability by ignoring these things.
With an open source projects build built on many different compilers with different optimizers it becomes even more important to adhere to strict standards. Fixing compiler warnings generated by multiple compilers provides even better coverage and this should be seen as an advantage and not a burden.
@Mark:
ReplyDeleteIf you have -Wall and -fno-strict-aliasing those warnings are still going to be thrown by the compiler. At least that is expected behavior, let me fire up a more standard linux enviroment and build a recent mysql and see for myself.
I get many type-punned warnings with "-Wall -O2". I get none with "-Wall -O2 -fno-strict-aliasing". This is with gcc 4.1.2.
ReplyDeleteA few years back we had intermittent crashes with MySQL 4.0 and strict-aliasing right after the optimization arrived in gcc.
The MySQL code base is full of casts back and forth between types, so it is not really suitable for using -fstrict-aliasing optimization. It either is ineffective (because too many pointers are cast to char*) or does faulty optimizations (because two pointers that are actually aliases are cast to different types).
ReplyDeleteI've proposed to use -fno-strict-aliasing for all code, but it needs to be evaluated carefully to avoid risks of regressions.
They should at least have a build bot that builds with -Wall and have a competent developer look over all the important warnings and chide the developer who introduced them.
ReplyDeleteBut in reality I think they should fix all the important warnings, create a tool to filter out known false positive warnings and have a policy which does not allow new code to be committed if it generates warnings, with very few exceptions.
It seems silly not to follow this practice.
Mark me down as another fan of -Wall, -Wextra, and -Werror. Where this becomes problematic is including 3rd party code (difficult or impossible to correct) and working with multiple compiler versions on multiple platforms. What will compile with one combo with these settings will fail on another and there may be no code change that will satisfy both versions (other then ifdefing on compiler version). Another annoyance is that turning off problematic warnings may cause some versions of a compiler to barf on unrecognized flags.
ReplyDeleteThis is probably a big reason why these things were made warning instead of errors in the first place.
My solution is to have a debug build that is strict and and a release build with flags that will hopefully work anywhere.
MySQL has made huge progress on this recently -- http://bugs.mysql.com/bug.php?id=53445
ReplyDelete