[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:567
       return Style.AllowShortCaseLabelsOnASingleLine
-                 ? tryMergeShortCaseLabels(I, E, Limit)
+                 ? tryMergeShortCaseLabels(I, E, Limit, PreviousLine, Style)
                  : 0;
----------------
See below.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:635-636
                           SmallVectorImpl<AnnotatedLine *>::const_iterator E,
-                          unsigned Limit) {
+                          unsigned Limit, const AnnotatedLine *PreviousLine,
+                          const FormatStyle &Style) {
     if (Limit == 0 || I + 1 == E ||
----------------
We don't need to add `Style`.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:643-644
       return 0;
+    bool NoFallThrough = Style.AllowShortCaseLabelsOnASingleLine ==
+                         FormatStyle::SCLS_NoFallThrough;
+    // Don't merge if the last thing on the line before was just `case X:`.
----------------
Nit.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:646
+    // Don't merge if the last thing on the line before was just `case X:`.
+    if (NoFallThrough && PreviousLine && PreviousLine->Last) {
+      if (PreviousLine->Last->is(tok::colon))
----------------
`Last` should never be null.


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:650-653
+      if (PreviousLine->Last->Previous && PreviousLine->Last->Previous) {
+        auto *PrevPrev = PreviousLine->Last->Previous->Previous;
+        if (PrevPrev && PrevPrev->startsSequence(TT_AttributeSquare,
+                                                 TT_AttributeSquare, tok::semi))
----------------
curdeius wrote:
> Haven't seen this before.
Use `endsSequence` instead.


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

https://reviews.llvm.org/D133571