From dev-return-6106-archive-asf-public=cust-asf.ponee.io@mxnet.incubator.apache.org Wed May 22 20:59:10 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id DDB93180651 for ; Wed, 22 May 2019 22:59:09 +0200 (CEST) Received: (qmail 32521 invoked by uid 500); 22 May 2019 20:59:09 -0000 Mailing-List: contact dev-help@mxnet.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@mxnet.incubator.apache.org Delivered-To: mailing list dev@mxnet.incubator.apache.org Received: (qmail 32508 invoked by uid 99); 22 May 2019 20:59:09 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 22 May 2019 20:59:09 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 7D66FC2E15 for ; Wed, 22 May 2019 20:59:08 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -0.2 X-Spam-Level: X-Spam-Status: No, score=-0.2 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd4-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id E26365WgoODb for ; Wed, 22 May 2019 20:59:06 +0000 (UTC) Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 5CE535FE68 for ; Wed, 22 May 2019 20:51:06 +0000 (UTC) Received: by mail-lf1-f43.google.com with SMTP id x132so2748470lfd.0 for ; Wed, 22 May 2019 13:51:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CbzuyJyi7/Ns9w2hElexlnh5Bw4ilDUqT6t6dwIucVg=; b=Q/70Bq8OJZFKD2lTSR6MUpOMquaZ1m/0kFjpQtl6T9485l2gT82MkQt2hu7jYG3hGW KRFTHnFSBcL//wTQLMfDB02gzgSruKtFL/v2gT5Zwtdd91l2z8ijyAI7oKIiN3+C9syC lvwUsxpbsFBdj6m2zsMA0fvvdyk8EdT9ftNxlfImOM43tU3oCoqY5sRPvBBTpvNKnRA4 Zu1i3Qh3IoinbHpyVw+k5AHgY38e4D8fmoaazJXL9BwcE+5xs6E4ppnhDFUhx8lnPCZw W9f8kaLZpNQbSQ++r3mXM1qsAsaNfc25KxnbyMewGNLRbBR1k4fiK3sCXXtu4jvNnj/q DXuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CbzuyJyi7/Ns9w2hElexlnh5Bw4ilDUqT6t6dwIucVg=; b=sIY97no1tzsYrUJ3hj6fo9mHG/0Qz4l2M8KvL9hYZxOG3z1snLbyp+d5OjCPG8qocb 5acGP80cgCCiYREwqBTwRBWSKN4L8IoOP885DOyLSAwZitMQYgAQsw4OhQiZfK0FL6nj O10VrGCNVziYXrRpfb9/BWhnkw6fU41jIfL6FWoKd2RVcLo/TkzWpQ1zgQqnuf9ORfU5 UU/50QNJI1uxQXJ6UiEiCDz84IMsk8BboaLhlZRz3w8AO4Jx7OfjDmjIhCR/GgE/onVI xXGuSc3n4f+60eP+X0FlyJ0obokrSH2ZSA7Io/y8G4HCGWplhplkZ0PW94uLgnoRfq1d tUqA== X-Gm-Message-State: APjAAAWWUqur0p0CH/o0RIgW5Uz2uXt6Atj9cGetYu++p2hbjCNiL/Ze TpD3UvknL1dhTV9cRGbJKYaJQ1NKXlhuhFou4dUHcpiWuzU= X-Google-Smtp-Source: APXvYqzphdXp5l/nWnn1Fw2xyg6DYqBvMWkNpAls/EYsh6CD0yzIwq2oDo62/GaZhVN3hnQnyv+fgpC1CmSYi64uBFM= X-Received: by 2002:ac2:4c93:: with SMTP id d19mr26108691lfl.116.1558558265430; Wed, 22 May 2019 13:51:05 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Pedro Larroy Date: Wed, 22 May 2019 13:50:53 -0700 Message-ID: Subject: Re: warnings as errors To: dev@mxnet.incubator.apache.org Cc: dev@mxnet.apache.org Content-Type: text/plain; charset="UTF-8" I was not able to fix the warnings on mshadow type switch with unused local typedefs, that's one example of warning that I would disable. I couldn't find a way to solve that one and I think the ramifications of an unused typedef are not likely to cause bugs in the code and are more of a pedantic nature. https://github.com/apache/incubator-mxnet/pull/13424 I think turning on them one by one is going to pollute the compilation output unecesarily and even run into commandline length problems? I think is best to enable all warnings and errors and cherry pick the ones we can't fix or won't fix on purpose. In this other case, I managed to tighten the warnings but ASAN is giving some problems: https://github.com/apache/incubator-mxnet/pull/14850 I think having warning fixes reviewed and merged faster without triggering additional refactorings could make this process easier, also having some help in this area and contributions would be greatly appreciated. Pedro. On Tue, May 21, 2019 at 3:49 PM Sheng Zha wrote: > > It would be great to enforce the check for warnings and treat as errors. Some questions I have: > - what are the warnings that you think should be ignored? > - for the rest of the warning types, can we turn them on one by one? > > -sz > > On 2019/05/21 22:33:51, Pedro Larroy wrote: > > Hi dev@ > > > > I try to fix any warning that I see during compilation of MXNet in my > > platform and with the build toggles that I care about. These seemingly > > trivial and ungrateful efforts, take nonetheless energy on the > > contributor side. > > > > I think overall I submitted myself more than a dozen of PRs fixing > > warnings and I would like to call for additional help and > > contributions in this area. > > > > There was a question from Lin about discussing this on the mailing > > list, I have the feeling that everybody agrees on moving towards zero > > warnings and warnings as errors. I think there are unavoidable > > warnings that can be disabled specifically such as the one triggered > > by mshadow type switch. > > > > Some important missing warnings such as warning on missing return > > values (ie. forgetting to return on a function returning non-void) > > cause bugs, danger and additional time spent bugfixing, which can be > > better spent somewhere else. > > > > Is there a process that we can figure out such as a more expedited > > merges of PRs fixing warnings or a specific label? > > > > Some simple PRs that fixes a warning can take long to merge, and > > sometimes trigger too much discussion and make the progress a bit > > unfriendly to contributors. > > > > Any help or constructive ideas on this topic would be appreciated. > > > > Pedro. > >