[PATCH] D119785: [clang-format] Fix formatting of struct-like records followed by variable declaration.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:470
+        ShouldMerge = Style.AllowShortEnumsOnASingleLine;
+      } else if (TheLine->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace)) {
+        // NOTE: We use AfterClass (whereas AfterStruct exists) for both classes
----------------
HazardyKnusperkeks wrote:
> Why not Union?
Well, it wasn't handled here before.
I haven't found a case that breaks with unions and didn't want to incorporate even more changes.
I'll be happy to do this later though, I'd like to understand first why below we need `!Style.BraceWrapping.AfterClass` (there is a dozen of failing tests if removed, also, it's equivalent to `!Style.BraceWrapping.AfterStruct` 😵).

Also, as you might guess, unions are handled in the `else` that handles functions, which is, yuck, not the best thing to put it mildly.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:735
         // We don't merge short records.
-        FormatToken *RecordTok = Line.First;
-        // Skip record modifiers.
-        while (RecordTok->Next &&
-               RecordTok->isOneOf(tok::kw_typedef, tok::kw_export,
-                                  Keywords.kw_declare, Keywords.kw_abstract,
-                                  tok::kw_default, Keywords.kw_override,
-                                  tok::kw_public, tok::kw_private,
-                                  tok::kw_protected, Keywords.kw_internal))
-          RecordTok = RecordTok->Next;
-        if (RecordTok &&
-            RecordTok->isOneOf(tok::kw_class, tok::kw_union, tok::kw_struct,
-                               Keywords.kw_interface))
+        if (isRecordLBrace(*Line.Last))
           return 0;
----------------
MyDeveloperDay wrote:
> wow these are equivalent? do we need to worry about trailing comments?
> 
> ```
> public class A { /* comment */
> ```
Yes, before we were doing a poor man's scan skipping some (but not all) keywords (hence the bugs) that could appear before a record.
That was error-prone and redundant w.r.t. the parsing done in UnwrappedLineParser.

Anyway, good catch, I'll test what happens with comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119785