[PATCH] D34176: [cmake] Make sure that -Wcast-qual is not a error in preparation for clang's -Wcast-qual for C++

[David Blaikie via llvm-commits]

I'd either keep the warning on or turn it off, not just turn it into a
warning rather than an error.

Is it impractical to cleanup the existing cases? (though I'm not opposed to
Justin's suggestion of grandfathering in projects that violate this - but
nicer to avoid it if it's not too much hassle to cleanup)

On Tue, Jun 13, 2017 at 4:40 PM Justin Bogner <mail at justinbogner.com> wrote:

> Roman Lebedev via Phabricator <reviews at reviews.llvm.org> writes:
> > lebedev.ri created this revision.
> > Herald added a subscriber: mgorny.
> >
> > Diagnostic `-Wcast-qual` is not enabled by default in gcc/clang, It
> > was enabled explicitly some time ago.
> > Later, clang has learned to do `-Wcast-qual`, like gcc does.
> > While that clang diagnostic is compatible with gcc's, it additionally
> > catches one more case, see https://reviews.llvm.org/D34153.
> > And last but not least, clang's diagnostic `-Wcast-qual` only works
> > for `C` code (sic!)
> > In https://reviews.llvm.org/D33102, i have extended it to work for
> > `C++` code too.
> > Once that landed, `-Werror` buildbots broke. That is when i was made
> > aware that this diagnostic is enabled in llvm builds.
> > I have fixed that particular buildbot-discovered failure in
> > https://reviews.llvm.org/D34153.
> > However, when i tried building clang stage2 with self-built clang
> > stage1 (with https://reviews.llvm.org/D33102), i have quickly
> > discovered that some sub-projects also exhibit this warning.
> > Specifically, **openmp** and **libcxx** subprojects produce **401**
> > -Wcast-qual warnings, and i //believe// not all of these warnings are
> > clang-specific.
> > (i don't think i have all subprojects checked-out, so there may be even
> more)
> > This tells two things:
> >
> > - these subprojects are not being built by `-Werror` buildbots. (at
> > least as part of LLVM, not standalone)
> > - no one builds LLVM by GCC with these subprojects with `-Werror`
> enabled.
> >
> > So, if https://reviews.llvm.org/D33102 is to land, this //may// not
> > break `-Werror` buildbots. This is bad and good.
> > But. If someone wants to build LLVM with these subprojects and with
> > `-Werror`, he/she/it will face a lot of fatal warnings.
> > It is unfeasible for me to fix all these warnings before re-landing
> > https://reviews.llvm.org/D33102, thus i propose this diff.
> >
> > After https://reviews.llvm.org/D33102 has landed, the buildbots should
> > //probably// be adjusted to catch these cases, and i will see if i can
> > help fix these issues and revert this patch.
> >
> > Thoughts?
>
> I'd rather we only set this to -Wno-error for the projects that actually
> have this warning, and we leave it as is for the projects that are
> already clean.
>
> > Repository:
> >   rL LLVM
> >
> > https://reviews.llvm.org/D34176
> >
> > Files:
> >   cmake/modules/HandleLLVMOptions.cmake
> >
> > Index: cmake/modules/HandleLLVMOptions.cmake
> > ===================================================================
> > --- cmake/modules/HandleLLVMOptions.cmake
> > +++ cmake/modules/HandleLLVMOptions.cmake
> > @@ -521,6 +521,7 @@
> >  if (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR
> CLANG_CL))
> >    append("-Wall -W -Wno-unused-parameter -Wwrite-strings" CMAKE_C_FLAGS
> CMAKE_CXX_FLAGS)
> >    append("-Wcast-qual" CMAKE_CXX_FLAGS)
> > +  append("-Wno-error=cast-qual" CMAKE_CXX_FLAGS)
> >
> >    # Turn off missing field initializer warnings for gcc to avoid noise
> from
> >    # false positives with empty {}. Turn them on otherwise (they're off
> by
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170614/64e7c068/attachment.html>