[PATCH] D116313: [MSVC] Silence -Wnon-virtual-dtor on DIA APIs

Wed Jan 5 09:00:21 PST 2022

dblaikie added a comment.

In D116313#3222508 <https://reviews.llvm.org/D116313#3222508>, @hans wrote:

> In D116313#3218224 <https://reviews.llvm.org/D116313#3218224>, @aganea wrote:
>
>> I was wondering, can we mandate `LLVM_ENABLE_WERROR=ON` for all bots at least? (or maybe enable it through cmake, if ever there's a setting to detect we're running as a bot)
>> Or just reverse the default setting on the main branch, ie. make `LLVM_ENABLE_WERROR=ON` by default and let users disable it locally. I think that would encourage developers to care more about warnings? Release branches could have it disabled somehow, to accommodate for the changing compiler environments. Was this discussed before?
>
> I don't remember whether this was discussed before. I think it would be a good topic for llvm-dev.
>
> It would be nice to have at least one buildbot using -Werror, with some specific blessed compiler version, but for that to not get annoying I think we'd need to surface those warnings in code reviews, and even if we have system a for that today it doesn't seem to be working very well.

Not sure we'd be much worse off than we are today if some buildbots enabled -Werror without phabricator pre-commit integration. I'm pretty sure we've had -Werror buildbots in the past in that situation. In some ways it'd be ideal if it were a bootstrap build so we ensure LLVM's always warning-clean with current Clang, but that means longer turnaround. (wonder if we could configure buildbots to fail on warnings, but not enable -Werror - so the buildbot would also still produce other useful results, rather than halting on the first warning & potentially hiding/delaying other useful findings)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116313/new/

https://reviews.llvm.org/D116313