This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Break non-trailing comments, try 2
ClosedPublic

Authored by krasimir on Sep 11 2017, 8:22 AM.

Details

Summary

This patch enables BreakableToken to manage the formatting of non-trailing
block comments. It is a refinement of https://reviews.llvm.org/D37007.
We discovered that the optimizer outsmarts us on cases where breaking the comment
costs considerably less than breaking after the comment. This patch addresses
this by ensuring that a newline is inserted between a block comment and the next
token.

Event Timeline

krasimir created this revision.Sep 11 2017, 8:22 AM
krasimir updated this revision to Diff 114617.Sep 11 2017, 8:23 AM
  • Add the original test cases
djasper added inline comments.Sep 26 2017, 7:05 AM
lib/Format/ContinuationIndenter.h
270

We should be *very* careful when adding stuff to ParenState as every extra bit of data and every extra comparison can have substantial cost.

Here, specifically:

  • Make this more generic. This currently addresses a very specific use case (comment broken), whereas the action is probably always going to be the same (enforce break). I think we should call this "NoContinuation" or "NeedsNewline" or something.
  • This seems always to only relate to the previous token. So it can be always reset when the state is moved further.
  • As this always refers to the previous token, this can probably live in LineState instead of ParenState. That way, it has less runtime/space overhead.
krasimir updated this revision to Diff 116742.Sep 26 2017, 5:47 PM
  • Change bool from Paren to Line and generalize it
krasimir marked an inline comment as done.Sep 26 2017, 5:47 PM
djasper accepted this revision.Oct 12 2017, 7:11 AM

looks good.

This revision is now accepted and ready to land.Oct 12 2017, 7:11 AM
This revision was automatically updated to reflect the committed changes.