[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

Nico Weber via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 8 07:29:13 PDT 2019


thakis added a comment.

In D68640#1699563 <https://reviews.llvm.org/D68640#1699563>, @gribozavr wrote:

> It looks to me that a better fix is to fix the checker to not emit this warning in MS compatibility mode.


The warning is about emitting "here's a redundant declaration", and the test expects an "extern inline" decl to be redundant with an "inline" definition. In ms compat mode they aren't, so the checker does not emit the warning in ms mode (but does elsewhere).

Arguably having a check that suggests removing a bunch of code that's necessary in some modes (ms compat, or C) is a bit weird, so maybe we should never emit this diag for extern inlines. I don't know which policy decisions clang-tidy usually makes for cross-platform development – does it prioritize cross-platform dev, or completeness assuming single-platform dev?

(Finally, these tests have been broken for months, folks are landing lots of stuff with "UNSUPPORTED: win32" (clang VFS patches recently, for example) and we're struggling just keeping tests green on Windows. There's no shortage of possible implicit TODOs :) I think it's better to land this to get the check-clang-tools target green than it is to mark the test UNSUPPORTED.)


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

https://reviews.llvm.org/D68640





More information about the cfe-commits mailing list