[PATCH] Don't merge constructors with initializer lists to one line in WebKit style.

Daniel Jasper djasper at google.com
Tue Dec 24 00:57:43 PST 2013



================
Comment at: lib/Format/Format.cpp:701
@@ +700,3 @@
+        for (unsigned i = 0; i < MergedLines && I + i + 1 != E; ++i)
+          I[i + 1]->First->MustBreakBefore = true;
+        MergedLines = 0;
----------------
Thinking some more about this, this is not enough. We must not modify the lines here as this method is potentially called many times for the same set of lines if they are children (i.e. in a nested block) of other lines. I cannot easily come up with a good example of when you'd have a constructor inside a nested block (e.g. DEBUG({..})), but I am sure somebody will.

So, we would need to guard this on DryRun, but this would (a) be horribly convoluted and (b) the calculation would be wrong (dryrun-mode and non-dryrun-mode would give a different penalty for the same solution). I will probably even harder to come up with an example of this, but I'd rather avoid it altogether.

I have sent a different approach in http://llvm-reviews.chandlerc.com/D2467. Please let me know what you think and especially whether I am missing something (I am quite confused with this entire back and forth on when to merge lines...).



http://llvm-reviews.chandlerc.com/D2455



More information about the cfe-commits mailing list