[PATCH] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 3 02:43:10 PDT 2020


MyDeveloperDay added a comment.

In D80950#2069733 <https://reviews.llvm.org/D80950#2069733>, @curdeius wrote:

> The change seems to me technically sound, but I'm not sure of the scope of its effects. There might be users that rely on this behavior. On the other hand, adding an option to keep the old behavior doesn't seem appropriate, and personally I consider the old behavior as a bug.


I also have concerns about my own fix. This will break existing code similar to that found in UnwrappedLineParser.cpp that I included here, whilst I also think this is a bug, I fear users will complain that we broke compatibility. The fact it lacked tests was also a shock given how vigorously that is defended when people add new functionality.

One thing that perhaps we could consider is keeping the old odd behavior but behind a new configuration option, declaring it an outright bug and removing it might just be a more subjective than an objective opinion (if I think about the longevity of this 7+year and remove my myopic view that its a bug I can see that some code could be made less readable by this change)

What do people think about if we added

  enum ChevronBreakingStyle
  {
      Never,  // at line limit only
      LegacyBetweenStrings, // (current)
      AfterEndOrNewLine,   // we could break on endl/std::endl/ends/std::ends/"\n" or any string literal ending with \n
      Always  // like between strings but between all  << types   and  << "string"
  }

Even if the default was LegacyBetweenStrings, I think those that are concerned about how it currently works would likely be happier to make the change to get the behavior they wanted, that way we wouldn't even break anywone.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80950/new/

https://reviews.llvm.org/D80950





More information about the cfe-commits mailing list