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

2019-10-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.llvm.org/D68640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8cb804a3c9ce: Try to get readability-deleted-default.cpp to pass on Windows. (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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

2019-10-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.llvm.org/D68640 ___ cfe

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 223917. thakis edited the summary of this revision. thakis added a comment. make test more explicit CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.llvm.org/D68640 Files: clang-tools-extra/test/clang-tidy/readability-redun

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. > Can you clarify what exactly the TODO is? As-is, the check suggests removing > the redeclaration where it's a no-op (non-ms) but not where it isn't (C, ms > compat). If I understand your reply correctly, this is desired behavior. Is > the TODO then to have test coverag

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. In D68640#1699628 , @aaron.ballman wrote: > In D68640#1699605 , @thakis wrote: > > > In D68640#1699563 , @gribozavr > > wrote: > > > > > It looks to

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

2019-10-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D68640#1699605 , @thakis wrote: > In D68640#1699563 , @gribozavr wrote: > > > It looks to me that a better fix is to fix the checker to not emit this > > warning in MS compatibilit

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

2019-10-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. > There's no shortage of possible implicit TODOs I don't see "-fno-ms-compatibility" as an implicit TODO. It is most commonly used as "this test does something that does not work in MS mode". When I read it, I can't tell why it is there. When other people write tests,

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

2019-10-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In 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. +1 > I'm OK with "fixing" the test like this, however, it should come wit

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. In 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 exp

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

2019-10-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. It looks to me that a better fix is to fix the checker to not emit this warning in MS compatibility mode. I'm OK with "fixing" the test like this, however, it should come with a TODO. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.l

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis created this revision. thakis added a reviewer: rnk. thakis added a comment. There's just one other use of `-fno-ms-compatibility` in clang-tidy's test suite, so I'm not 100% sure this is the way to go. It's better than disabling the test completely on Windows :) In MS compatibility mod

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

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. There's just one other use of `-fno-ms-compatibility` in clang-tidy's test suite, so I'm not 100% sure this is the way to go. It's better than disabling the test completely on Windows :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.l