[PATCH] D24501: Remove redundant comma around parenthesis in parameter list.

[Daniel Jasper via cfe-commits]

djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Some remarks, but looks good.


================
Comment at: unittests/Format/CleanupTest.cpp:38
@@ +37,3 @@
+  // Returns code after cleanup around \p Offsets.
+  std::string cleanupAroundOffsets(llvm::ArrayRef<unsigned int> Offsets,
+                              llvm::StringRef Code) {
----------------
I thin we normally just used "unsigned" instead of "unsigned int".

================
Comment at: unittests/Format/CleanupTest.cpp:39
@@ +38,3 @@
+  std::string cleanupAroundOffsets(llvm::ArrayRef<unsigned int> Offsets,
+                              llvm::StringRef Code) {
+    std::vector<tooling::Range> Ranges;
----------------
nit: alignment.

================
Comment at: unittests/Format/CleanupTest.cpp:125
@@ -123,7 +124,3 @@
   std::string Expected = "class A {\nA()  {} };";
-  std::vector<tooling::Range> Ranges;
-  Ranges.push_back(tooling::Range(17, 0));
-  Ranges.push_back(tooling::Range(19, 0));
-  std::string Result = cleanup(Code, Ranges);
-  EXPECT_EQ(Expected, Result);
+  EXPECT_EQ(Expected, cleanupAroundOffsets({17, 19}, Code));
 
----------------
I think for such short test cases, at least, I'd just inline the string literals, e.g.:

  EXPECT_EQ("class A {\nA()  {} };",
            cleanupAroundOffsets({17, 19}, "class A {\nA() : , {} };"));

But not sure whether it's better.


https://reviews.llvm.org/D24501