[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3669-3688
+    bool lineContainsBreakingTokens = false;
+    const FormatToken *breakingSearchToken = &Right;
+    while ((breakingSearchToken = breakingSearchToken->Next)) {
+      bool hasBreakingComma = breakingSearchToken->is(tok::comma) &&
+                              breakingSearchToken->Next->is(tok::r_brace);
+      if (breakingSearchToken->isTrailingComment() || hasBreakingComma) {
+        lineContainsBreakingTokens = true;
----------------
Please refactor this part so that the conditions can short-circuit, and start with checking for `!Style.AllowShortEnumsOnASingleLine` (the cheapest check) and end with the `while` loop (the apparently most expensive check).
You can do this by putting these conditions in a lambda.
Something like:

```
auto isAllowedByShortEnums = [&] () {
  if (!Style.AllowShortEnumsOnASingleLine) return false;

  // check isLineTooBig etc.

    const FormatToken *breakingSearchToken = &Right;
    while ((breakingSearchToken = breakingSearchToken->Next)) {
      bool hasBreakingComma = breakingSearchToken->is(tok::comma) &&
                              breakingSearchToken->Next->is(tok::r_brace);
      if (breakingSearchToken->isTrailingComment() || hasBreakingComma) {
        return true;
      }
    }

  return false;
};
return (isAllowedByAfterEnum && isAllowedByShortEnums()) || // ...
```


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3683
+        Style.BraceWrapping.AfterEnum;
+    bool isLineTooBig = (strlen(Right.TokenText.data()) +
+                         Right.OriginalColumn) > Style.ColumnLimit;
----------------
strlen? Please use `StringRef::size()`.


================
Comment at: clang/unittests/Format/FormatTest.cpp:13319
   AllmanBraceStyle.BreakBeforeBraces = FormatStyle::BS_Allman;
+  AllmanBraceStyle.AllowShortEnumsOnASingleLine = false;
 
----------------
That shouldn't be necessary here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938