aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115 +// either due to crashes or false positives. +const char *getClangTidyBlacklist() { + static const std::string FalsePositives = ---------------- kadircet wrote: > sammccall wrote: > > aaron.ballman wrote: > > > kadircet wrote: > > > > aaron.ballman wrote: > > > > > njames93 wrote: > > > > > > Return by StringRef? > > > > > How about `getDisabledClangTidyChecks()` (or literally any other name > > > > > than blacklist)? > > > > thanks for bringing this to my attention, i will try to be more > > > > conscious next time. > > > > > > > > I would prefer allow/deny as `disabled` might also be offensive in some > > > > contexts. Do you know if we already have some settlements around this > > > > one in the wider community? > > > > thanks for bringing this to my attention, i will try to be more > > > > conscious next time. > > > > > > No worries! > > > > > > > I would prefer allow/deny as disabled might also be offensive in some > > > > contexts. Do you know if we already have some settlements around this > > > > one in the wider community? > > > > > > I don't believe there's any consensus around avoiding use of "disabled" > > > (we use the term in a lot of places, especially when paired with > > > "enabled"), but I'd also be fine with allow/deny terminology instead. > > > > > > As a minor drive-by comment, the function should also be marked `static` > > > and placed outside of the anonymous namespace (per our usual coding > > > style). > > I dislike "disabled" because it's vague: every check that's not enabled is > > disabled, but only a few of them are mentioned here. > > > > I'd suggest BadlyBehavedTidyChecks or UnusableTidyChecks... > > As a minor drive-by comment, the function should also be marked static and > > placed outside of the anonymous namespace (per our usual coding style). > > That's what we do when working in llvm and clang, but convention around > clangd is putting implementation details in anon namespaces. I would rather > not stray from it here. > > > As for the general naming issues, i went with `UnusableTidyChecks`. > That's what we do when working in llvm and clang, but convention around > clangd is putting implementation details in anon namespaces. I would rather > not stray from it here. It's unfortunate that clangd decided to stray from the community standards like this, but thank you for letting me know -- I agree it should match the local style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83224/new/ https://reviews.llvm.org/D83224 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits