Page MenuHomePhabricator

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

Authored by lebedev.ri on Jun 13 2017, 3:07 PM.

Details

Summary

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 D34153.
And last but not least, clang's diagnostic -Wcast-qual only works for C code (sic!)
In 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 D34153.
However, when i tried building clang stage2 with self-built clang stage1 (with 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 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 D33102, thus i propose this diff.

After 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?

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Jun 13 2017, 3:07 PM

As of r305410, libc++ passes all the tests w/ -Wcast-qual enabled.

Ok, so i suppose libc++ is fine now, thanks to @mclow.lists
I can not build lldb, so i can not check whether it has similar issues.
This leaves only the OpenMP?

To reiterate, there are several solutions:

  1. Do nothing, just land D33102, i'm under impression no current buildbot will break. That, however, will break anyone who builds with openmp subproject and with -DLLVM_ENABLE_WERROR=ON
  2. Disable -Wcast-qual in openmp
  3. This approach, which is wrong - disable it globally
  4. Wait until openmp is fixed. I suppose i could help, though it is really better if someone with the actual knowledge about the code fixes these warnings...

I'd really like to hear any thoughts from the openmp people :)

I will try to make cleanup of the OpenMP runtime sources, hopefully this week.

  • Andrey

I will try to make cleanup of the OpenMP runtime sources,

That would be lovely :)
I sure can try to help, if needed.

hopefully this week.

  • Andrey

Will be superseded by D34759

lebedev.ri abandoned this revision.Jul 3 2017, 7:43 AM

Abandoning.
All preparatory fixes has landed.