arrow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Micah Kornfield <emkornfi...@gmail.com>
Subject Re: [C++] BUILD_WARNING_LEVEL=EVERYTHING?
Date Tue, 05 Mar 2019 06:11:48 GMT
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 <wesmckinn@gmail.com> wrote:

> No opposition from me
>
> On Sun, Mar 3, 2019 at 10:02 PM Micah Kornfield <emkornfield@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 <wesmckinn@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 <antoine@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
> > > > >
> > >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message