Hi. These two prs are fixing warnings on different platforms and removing some code bloat due to inlines. Could you guys help get them in? they are open for a while.
https://github.com/apache/incubator-mxnet/pull/15270 https://github.com/apache/incubator-mxnet/pull/14940 Thanks. Pedro. On Wed, May 22, 2019 at 1:50 PM Pedro Larroy <[email protected]> wrote: > > 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 <[email protected]> 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 <[email protected]> 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. > > >
