After spending a few hours trying to fix the warnings I think I've come around to your points of view :)
What got me in the end is DCHECK macros aren't allowed in headers (and I think if we want to clean these up, we should DCHECK before doing casts). Cheers, Micah On Sun, Mar 3, 2019 at 8:11 PM Wes McKinney <wesmck...@gmail.com> wrote: > No opposition from me > > On Sun, Mar 3, 2019 at 10:02 PM Micah Kornfield <emkornfi...@gmail.com> > wrote: > > > > I'm ok with that. I think some of the conversion warnings might be > useful > > (I know I've had bugs in other code that would have been caught with > > them). Would people be opposed if I tried to go through and cleanup the > > EVERYTHING warnings even if more might creep in? > > > > Thanks, > > Micah > > > > On Sun, Mar 3, 2019 at 3:27 PM Wes McKinney <wesmck...@gmail.com> wrote: > > > > > I'm of the same mind as Antoine on this. I think it's useful to look > > > at the EVERYTHING warnings periodically, but it is enough effort to > > > keep things simultaneously building cleanly with gcc, clang, and MSVC, > > > that I would prefer to maintain the status quo until it can be > > > demonstrated to be a problem (and even then, it just might be that we > > > add more specific warnings that we care about to the CHECKIN warning > > > level). The clang CHECKIN warnings catch some definitely bad things > > > like missing virtual dtors etc. > > > > > > - Wes > > > > > > On Sun, Mar 3, 2019 at 3:38 AM Antoine Pitrou <anto...@python.org> > wrote: > > > > > > > > > > > > Hmm... There are enough warnings that need pampering in the default > > > > settings that I don't think we want to go the full length of enabling > > > > all warnings. Sometimes it's a PITA to get code to compile cleanly > on > > > > all platforms. > > > > > > > > If compiler writers had a more reasonable judgement when it comes to > > > > designing and enabling warnings, I would perhaps revise my position > ;-) > > > > > > > > Regards > > > > > > > > Antoine. > > > > > > > > > > > > Le 03/03/2019 à 04:47, Micah Kornfield a écrit : > > > > > As part of trying to fix the mingw C++ build [1], I tried compiling > > > with > > > > > BUILD_WARNING_LEVEL=EVERYTHING and it seems like it highlights a > lot of > > > > > possible warnings that aren't in CHECKIN. Have we not turned on > the > > > > > additional warnings because there was too much to fix at the time > this > > > was > > > > > added? Or is a conscious decision to ignore some warnings? > > > > > > > > > > Thanks, > > > > > Micah > > > > > > > > > > [1] https://github.com/apache/arrow/pull/3793 > > > > > > > > >
