[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: unittests/clang-tidy/ClangTidyTest.h:159-160
+      CleannedReplacements = std::move(FormattedReplacements);
+      if (!CleannedReplacements)
+        llvm_unreachable("!CleannedReplacements");
+    } else {
----------------
jdemeule wrote:
> aaron.ballman wrote:
> > This smells like it should be an assert rather than an unreachable.
> Is returning an empty string OK?
> This should let the assertion on the test do the job.
> Is returning an empty string OK?

It seems like a reasonable behavior, but I've not tried it myself.

> This should let the assertion on the test do the job.

If an assertion fails with assertions disabled, the resulting UB is generally acceptable. Because these are unit tests, I think we can generally rely on the assertion being enabled if this code is being run.


================
Comment at: unittests/clang-tidy/ClangTidyTest.h:167
+    if (!Result) {
+      // FIXME: propogate the error.
+      llvm::consumeError(Result.takeError());
----------------
jdemeule wrote:
> aaron.ballman wrote:
> > Are you intending to implement this fixme in this patch?
> I suggest to remove the copy/paste containing the fixme. Is-it OK?
> Maybe with some hints I can suggest a fix for this later.
Ah, I didn't see that this was a copy paste from below. I think those code paths could be combined if you hoisted the code out to the function level.


https://reviews.llvm.org/D43500