[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

Kirill Bobyrev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 12 06:29:56 PDT 2018


kbobyrev added inline comments.


================
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:35
+
+  std::string TypeAndName =
+      VarType.getAsString(TypePrinter) + " " + D->getNameAsString();
----------------
`llvm::Twine` here? Seems like this one is used a lot for concatenation, so just `.str()` when returning.


================
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:51
+
+    return TypeAndName + " = " + Initializer + ";";
+  }
----------------
This seems to replace `int x = 5, y = 42;` with `int x = 5;int y = 42`. I don't think that it becomes cleaner (in fact, without spaces in between it looks cryptic). Consider formatting it or simply applying newlines (if there were no newlines inbetween before).


================
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:71
+  auto Diag =
+      diag(WholeDecl->getBeginLoc(), "make only one declaration per statement");
+
----------------
Maybe it's just me: this doesn't seem like a very clear diagnostic message, probably needs better wording (can't think of anything in particular ATM, though).


================
Comment at: clang-tidy/readability/IsolateDeclCheck.h:1
+//===--- IsolateDeclCheck.h - clang-tidy-------------------------*- C++ -*-===//
+//
----------------
nit: space between clang-tidy (also, in .cpp file)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949





More information about the cfe-commits mailing list