aaron.ballman added a comment. In D68640#1699605 <https://reviews.llvm.org/D68640#1699605>, @thakis wrote:
> 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? I think it depends on the check, but for a check in the `readability` module, I'm not certain there's a clear answer. My gut feeling is to diagnose based on platform behavior because the goal is to remove redundancy and that requires platform-specific knowledge. But it's not a strong opinion. > (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.) To be clear, I wasn't suggesting we fix it in this patch, just that we add a FIXME/TODO to the test and make sure we're tracking the bug. Explicit TODO > implicit TODO. 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.org/cgi-bin/mailman/listinfo/cfe-commits
