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

[Ally Tiritoglu via Phabricator via cfe-commits]

atirit added a comment.

> This seems to be that in "Attach" mode, then AllowShortEnumsOnASingleLine=false doesn't attach the brace.

That is correct, but the main issue is that `AfterEnum: false`, which Attach mode implies, doesn't function correctly.

> Said all that, it *seems* to me that the fix is correct apart from the strangely looking if (!Style.isCpp()) { change that I don't really understand. Why should C++ be handled differently in this regard? What am I missing?

That code was there before. Here is the code currently in release:

  case tok::kw_enum:
    // Ignore if this is part of "template <enum ...".
    if (Previous && Previous->is(tok::less)) {
      nextToken();
      break;
    }
  
    // parseEnum falls through and does not yet add an unwrapped line as an
    // enum definition can start a structural element.
    if (!parseEnum())
      break;
    // This only applies for C++.
    if (!Style.isCpp()) {
      addUnwrappedLine();
      return;
    }
    break;

I modified this code in an attempt to fix this bug, but that broke styling in other languages. It turns out this section required no modification. The only changes my revisions at present introduce is the change for the unit test and the modification of the following function:

  static bool ShouldBreakBeforeBrace(const FormatStyle &Style,
                                     const FormatToken &InitialToken) {
    if (InitialToken.isOneOf(tok::kw_namespace, TT_NamespaceMacro))
      return Style.BraceWrapping.AfterNamespace;
    if (InitialToken.is(tok::kw_class))
      return Style.BraceWrapping.AfterClass;
    if (InitialToken.is(tok::kw_union))
      return Style.BraceWrapping.AfterUnion;
    if (InitialToken.is(tok::kw_struct))        // NEW
      return Style.BraceWrapping.AfterStruct;   // NEW
    return false;
  }

I can add some unit tests for the variations of `AllowShortEnumsOnASingleLine` and `AfterEnum`.

If there's anything I can explain better please let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938