[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

Tue Jan 7 08:21:43 PST 2020

JonasToth added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:272
+      if (const auto *Typedef =
+              Value->getReturnType().getTypePtr()->getAs<TypedefType>()) {
+        addUsage(NamingCheckFailures, Typedef->getDecl(),
----------------
aaron.ballman wrote:
> logan-5 wrote:
> > Eugene.Zelenko wrote:
> > > Please elide braces.
> > Would you mind pointing me toward a resource for these formatting nits? I don't see anything about requiring omitting braces for single-statement if()s in the official LLVM coding standards (and I happen to think the braces actually help readability here). If this stuff is explicitly documented somewhere and painstakingly enforced like this, I'd rather get it right the first time.
> I looked through the coding standard and it does not suggest this, which I am really surprised to see (I feel like the standard used to say it at some point, but it likely got removed and I did not notice). This is a common pattern that's used all over the code base (likely from fairly consistent review guidance over the years), and so it's best to be consistent with the style used locally, but that's a pretty weak argument for new code.
Did it maybe get dropped? Wasn't there are overhaul a few weeks ago? But I am surprised as well, as this was at one point explicitly stated.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284