Revert "clang-format: support aligned nested conditionals formatting"

This reverts 3d61b11, 5daa25f

The clang-format test (FormatTest.ConfigurableUseOfTab) is failing in the buildbot:

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/31811/steps/ninja%20check%201/logs/stdio

Author: hokein

clang/docs/ClangFormatStyleOptions.rst

@@ -270,49 +270,17 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**AlignOperands** (``OperandAlignmentStyle``)
+**AlignOperands** (``bool``)
   If ``true``, horizontally align operands of binary and ternary
   expressions.
 
-  Possible values:
-
-  * ``OAS_DontAlign`` (in configuration: ``DontAlign``)
-    Do not align operands of binary and ternary expressions.
-    The wrapped lines are indented ``ContinuationIndentWidth`` spaces from
-    the start of the line.
-
-  * ``OAS_Align`` (in configuration: ``Align``)
-    Horizontally align operands of binary and ternary expressions.
-
-    Specifically, this aligns operands of a single expression that needs
-    to be split over multiple lines, e.g.:
-
-    .. code-block:: c++
-
-      int aaa = bbbbbbbbbbbbbbb +
-                ccccccccccccccc;
-
-    When ``BreakBeforeBinaryOperators`` is set, the wrapped operator is
-    aligned with the operand on the first line.
-
-    .. code-block:: c++
-
-      int aaa = bbbbbbbbbbbbbbb
-                + ccccccccccccccc;
-
-  * ``OAS_AlignAfterOperator`` (in configuration: ``AlignAfterOperator``)
-    Horizontally align operands of binary and ternary expressions.
-
-    This is similar to ``AO_Align``, except when
-    ``BreakBeforeBinaryOperators`` is set, the operator is un-indented so
-    that the wrapped operand is aligned with the operand on the first line.
-
-    .. code-block:: c++
-
-      int aaa = bbbbbbbbbbbbbbb
-              + ccccccccccccccc;
+  Specifically, this aligns operands of a single expression that needs to be
+  split over multiple lines, e.g.:
 
+  .. code-block:: c++
 
+    int aaa = bbbbbbbbbbbbbbb +
+              ccccccccccccccc;
 
 **AlignTrailingComments** (``bool``)
   If ``true``, aligns trailing comments.

clang/include/clang/Format/Format.h

@@ -153,43 +153,16 @@ struct FormatStyle {
   /// Options for aligning backslashes in escaped newlines.
   EscapedNewlineAlignmentStyle AlignEscapedNewlines;
 
-  /// Different styles for aligning operands.
-  enum OperandAlignmentStyle {
-    /// Do not align operands of binary and ternary expressions.
-    /// The wrapped lines are indented ``ContinuationIndentWidth`` spaces from
-    /// the start of the line.
-    OAS_DontAlign,
-    /// Horizontally align operands of binary and ternary expressions.
-    ///
-    /// Specifically, this aligns operands of a single expression that needs
-    /// to be split over multiple lines, e.g.:
-    /// \code
-    ///   int aaa = bbbbbbbbbbbbbbb +
-    ///             ccccccccccccccc;
-    /// \endcode
-    ///
-    /// When ``BreakBeforeBinaryOperators`` is set, the wrapped operator is
-    /// aligned with the operand on the first line.
-    /// \code
-    ///   int aaa = bbbbbbbbbbbbbbb
-    ///             + ccccccccccccccc;
-    /// \endcode
-    OAS_Align,
-    /// Horizontally align operands of binary and ternary expressions.
-    ///
-    /// This is similar to ``AO_Align``, except when
-    /// ``BreakBeforeBinaryOperators`` is set, the operator is un-indented so
-    /// that the wrapped operand is aligned with the operand on the first line.
-    /// \code
-    ///   int aaa = bbbbbbbbbbbbbbb
-    ///           + ccccccccccccccc;
-    /// \endcode
-    OAS_AlignAfterOperator,
-  };
-
   /// If ``true``, horizontally align operands of binary and ternary
   /// expressions.
-  OperandAlignmentStyle AlignOperands;
+  ///
+  /// Specifically, this aligns operands of a single expression that needs to be
+  /// split over multiple lines, e.g.:
+  /// \code
+  ///   int aaa = bbbbbbbbbbbbbbb +
+  ///             ccccccccccccccc;
+  /// \endcode
+  bool AlignOperands;
 
   /// If ``true``, aligns trailing comments.
   /// \code

clang/lib/Format/ContinuationIndenter.cpp

@@ -367,12 +367,6 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
       State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
       !Current.isOneOf(tok::r_paren, tok::r_brace))
     return true;
-  if (State.Stack.back().IsChainedConditional &&
-      ((Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&
-            Current.is(tok::colon)) ||
-       (!Style.BreakBeforeTernaryOperators && Previous.is(TT_ConditionalExpr) &&
-            Previous.is(tok::colon))))
-    return true;
   if (((Previous.is(TT_DictLiteral) && Previous.is(tok::l_brace)) ||
        (Previous.is(TT_ArrayInitializerLSquare) &&
         Previous.ParameterCount > 1) ||
@@ -693,9 +687,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
     // does not help.
     bool HasTwoOperands =
         P->OperatorIndex == 0 && !P->NextOperator && !P->is(TT_ConditionalExpr);
-    if ((!BreakBeforeOperator &&
-         !(HasTwoOperands &&
-           Style.AlignOperands != FormatStyle::OAS_DontAlign)) ||
+    if ((!BreakBeforeOperator && !(HasTwoOperands && Style.AlignOperands)) ||
         (!State.Stack.back().LastOperatorWrapped && BreakBeforeOperator))
       State.Stack.back().NoLineBreakInOperand = true;
   }
@@ -1030,23 +1022,8 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   if (State.Stack.back().QuestionColumn != 0 &&
       ((NextNonComment->is(tok::colon) &&
         NextNonComment->is(TT_ConditionalExpr)) ||
-       Previous.is(TT_ConditionalExpr))) {
-    if (((NextNonComment->is(tok::colon) && NextNonComment->Next &&
-          !NextNonComment->Next->FakeLParens.empty() &&
-          NextNonComment->Next->FakeLParens.back() == prec::Conditional) ||
-         (Previous.is(tok::colon) && !Current.FakeLParens.empty() &&
-          Current.FakeLParens.back() == prec::Conditional)) &&
-        !State.Stack.back().IsWrappedConditional) {
-      //NOTE: we may tweak this slightly:
-      //    * not remove the 'lead' ContinuationIndentWidth
-      //    * always un-indent by the operator when BreakBeforeTernaryOperators=true
-      unsigned Indent = State.Stack.back().Indent - Style.ContinuationIndentWidth;
-      if (Style.BreakBeforeTernaryOperators && State.Stack.back().UnindentOperator)
-        Indent -= 2;
-      return Indent;
-    }
+       Previous.is(TT_ConditionalExpr)))
     return State.Stack.back().QuestionColumn;
-  }
   if (Previous.is(tok::comma) && State.Stack.back().VariablePos != 0)
     return State.Stack.back().VariablePos;
   if ((PreviousNonComment &&
@@ -1122,13 +1099,6 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
     return ContinuationIndent;
   if (Current.is(TT_ProtoExtensionLSquare))
     return State.Stack.back().Indent;
-  if (Current.isBinaryOperator() && State.Stack.back().UnindentOperator)
-    return State.Stack.back().Indent - Current.Tok.getLength() -
-           Current.SpacesRequiredBefore;
-  if (Current.isOneOf(tok::comment, TT_BlockComment, TT_LineComment) &&
-      NextNonComment->isBinaryOperator() && State.Stack.back().UnindentOperator)
-    return State.Stack.back().Indent - NextNonComment->Tok.getLength() -
-           NextNonComment->SpacesRequiredBefore;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
       !PreviousNonComment->isOneOf(tok::r_brace, TT_CtorInitializerComma))
     // Ensure that we fall back to the continuation indent width instead of
@@ -1174,10 +1144,6 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
   if (Current.is(TT_ArraySubscriptLSquare) &&
       State.Stack.back().StartOfArraySubscripts == 0)
     State.Stack.back().StartOfArraySubscripts = State.Column;
-  if (Current.is(TT_ConditionalExpr) && Current.is(tok::question) &&
-      ((Current.MustBreakBefore) ||
-       (Current.getNextNonComment() && Current.getNextNonComment()->MustBreakBefore)))
-    State.Stack.back().IsWrappedConditional = true;
   if (Style.BreakBeforeTernaryOperators && Current.is(tok::question))
     State.Stack.back().QuestionColumn = State.Column;
   if (!Style.BreakBeforeTernaryOperators && Current.isNot(tok::colon)) {
@@ -1308,7 +1274,7 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
       (Previous && (Previous->opensScope() ||
                     Previous->isOneOf(tok::semi, tok::kw_return) ||
                     (Previous->getPrecedence() == prec::Assignment &&
-                     Style.AlignOperands != FormatStyle::OAS_DontAlign) ||
+                     Style.AlignOperands) ||
                     Previous->is(TT_ObjCMethodExpr)));
   for (SmallVectorImpl<prec::Level>::const_reverse_iterator
            I = Current.FakeLParens.rbegin(),
@@ -1318,9 +1284,6 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
     NewParenState.Tok = nullptr;
     NewParenState.ContainsLineBreak = false;
     NewParenState.LastOperatorWrapped = true;
-    NewParenState.IsChainedConditional = false;
-    NewParenState.IsWrappedConditional = false;
-    NewParenState.UnindentOperator = false;
     NewParenState.NoLineBreak =
         NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
@@ -1332,26 +1295,14 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
     // a builder type call after 'return' or, if the alignment after opening
     // brackets is disabled.
     if (!Current.isTrailingComment() &&
-        (Style.AlignOperands != FormatStyle::OAS_DontAlign ||
-         *I < prec::Assignment) &&
+        (Style.AlignOperands || *I < prec::Assignment) &&
         (!Previous || Previous->isNot(tok::kw_return) ||
          (Style.Language != FormatStyle::LK_Java && *I > 0)) &&
         (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
-         *I != prec::Comma || Current.NestingLevel == 0)) {
+         *I != prec::Comma || Current.NestingLevel == 0))
       NewParenState.Indent =
           std::max(std::max(State.Column, NewParenState.Indent),
                    State.Stack.back().LastSpace);
-    }
-
-    // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-    // the operator and keep the operands aligned
-    if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator && Previous &&
-        (Previous->getPrecedence() == prec::Assignment ||
-         Previous->is(tok::kw_return) ||
-         (*I == prec::Conditional && Previous->is(tok::question) &&
-          Previous->is(TT_ConditionalExpr))) &&
-        !Newline)
-      NewParenState.UnindentOperator = true;
 
     // Do not indent relative to the fake parentheses inserted for "." or "->".
     // This is a special case to make the following to statements consistent:
@@ -1365,21 +1316,14 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
         Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
       NewParenState.StartOfFunctionCall = State.Column;
 
-    // Indent conditional expressions, unless they are chained "else-if"
-    // conditionals. Never indent expression where the 'operator' is ',', ';' or
-    // an assignment (i.e. *I <= prec::Assignment) as those have different
-    // indentation rules. Indent other expression, unless the indentation needs
-    // to be skipped.
-    if (*I == prec::Conditional && Previous && Previous->is(tok::colon) &&
-        Previous->is(TT_ConditionalExpr) && I == Current.FakeLParens.rbegin() &&
-        !State.Stack.back().IsWrappedConditional) {
-      NewParenState.IsChainedConditional = true;
-      NewParenState.UnindentOperator = State.Stack.back().UnindentOperator;
-    } else if (*I == prec::Conditional ||
-               (!SkipFirstExtraIndent && *I > prec::Assignment &&
-                !Current.isTrailingComment())) {
+    // Always indent conditional expressions. Never indent expression where
+    // the 'operator' is ',', ';' or an assignment (i.e. *I <=
+    // prec::Assignment) as those have different indentation rules. Indent
+    // other expression, unless the indentation needs to be skipped.
+    if (*I == prec::Conditional ||
+        (!SkipFirstExtraIndent && *I > prec::Assignment &&
+         !Current.isTrailingComment()))
       NewParenState.Indent += Style.ContinuationIndentWidth;
-    }
     if ((Previous && !Previous->opensScope()) || *I != prec::Comma)
       NewParenState.BreakBeforeParameter = false;
     State.Stack.push_back(NewParenState);

clang/lib/Format/ContinuationIndenter.h

@@ -202,15 +202,14 @@ struct ParenState {
   ParenState(const FormatToken *Tok, unsigned Indent, unsigned LastSpace,
              bool AvoidBinPacking, bool NoLineBreak)
       : Tok(Tok), Indent(Indent), LastSpace(LastSpace),
-        NestedBlockIndent(Indent), BreakBeforeClosingBrace(false),
-        AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),
-        NoLineBreak(NoLineBreak), NoLineBreakInOperand(false),
-        LastOperatorWrapped(true), ContainsLineBreak(false),
-        ContainsUnwrappedBuilder(false), AlignColons(true),
-        ObjCSelectorNameFound(false), HasMultipleNestedBlocks(false),
-        NestedBlockInlined(false), IsInsideObjCArrayLiteral(false),
-        IsCSharpGenericTypeConstraint(false), IsChainedConditional(false),
-        IsWrappedConditional(false), UnindentOperator(false) {}
+        NestedBlockIndent(Indent), IsAligned(false),
+        BreakBeforeClosingBrace(false), AvoidBinPacking(AvoidBinPacking),
+        BreakBeforeParameter(false), NoLineBreak(NoLineBreak),
+        NoLineBreakInOperand(false), LastOperatorWrapped(true),
+        ContainsLineBreak(false), ContainsUnwrappedBuilder(false),
+        AlignColons(true), ObjCSelectorNameFound(false),
+        HasMultipleNestedBlocks(false), NestedBlockInlined(false),
+        IsInsideObjCArrayLiteral(false), IsCSharpGenericTypeConstraint(false) {}
 
   /// \brief The token opening this parenthesis level, or nullptr if this level
   /// is opened by fake parenthesis.
@@ -336,18 +335,6 @@ struct ParenState {
 
   bool IsCSharpGenericTypeConstraint : 1;
 
-  /// \brief true if the current \c ParenState represents the false branch of
-  /// a chained conditional expression (e.g. else-if)
-  bool IsChainedConditional : 1;
-
-  /// \brief true if there conditionnal was wrapped on the first operator (the
-  /// question mark)
-  bool IsWrappedConditional : 1;
-
-  /// \brief Indicates the indent should be reduced by the length of the
-  /// operator.
-  bool UnindentOperator : 1;
-
   bool operator<(const ParenState &Other) const {
     if (Indent != Other.Indent)
       return Indent < Other.Indent;
@@ -389,12 +376,6 @@ struct ParenState {
       return NestedBlockInlined;
     if (IsCSharpGenericTypeConstraint != Other.IsCSharpGenericTypeConstraint)
       return IsCSharpGenericTypeConstraint;
-    if (IsChainedConditional != Other.IsChainedConditional)
-      return IsChainedConditional;
-    if (IsWrappedConditional != Other.IsWrappedConditional)
-      return IsWrappedConditional;
-    if (UnindentOperator != Other.UnindentOperator)
-      return UnindentOperator;
     return false;
   }
 };

clang/lib/Format/Format.cpp

@@ -308,18 +308,6 @@ struct ScalarEnumerationTraits<FormatStyle::EscapedNewlineAlignmentStyle> {
   }
 };
 
-template <> struct ScalarEnumerationTraits<FormatStyle::OperandAlignmentStyle> {
-  static void enumeration(IO &IO, FormatStyle::OperandAlignmentStyle &Value) {
-    IO.enumCase(Value, "DontAlign", FormatStyle::OAS_DontAlign);
-    IO.enumCase(Value, "Align", FormatStyle::OAS_Align);
-    IO.enumCase(Value, "AlignAfterOperator", FormatStyle::OAS_AlignAfterOperator);
-
-    // For backward compatibility.
-    IO.enumCase(Value, "true", FormatStyle::OAS_Align);
-    IO.enumCase(Value, "false", FormatStyle::OAS_DontAlign);
-  }
-};
-
 template <> struct ScalarEnumerationTraits<FormatStyle::PointerAlignmentStyle> {
   static void enumeration(IO &IO, FormatStyle::PointerAlignmentStyle &Value) {
     IO.enumCase(Value, "Middle", FormatStyle::PAS_Middle);
@@ -756,7 +744,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
-  LLVMStyle.AlignOperands = FormatStyle::OAS_Align;
+  LLVMStyle.AlignOperands = true;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
@@ -955,7 +943,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
 
   if (Language == FormatStyle::LK_Java) {
     GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-    GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
+    GoogleStyle.AlignOperands = false;
     GoogleStyle.AlignTrailingComments = false;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
     GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
@@ -966,7 +954,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
     GoogleStyle.SpacesBeforeTrailingComments = 1;
   } else if (Language == FormatStyle::LK_JavaScript) {
     GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
-    GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
+    GoogleStyle.AlignOperands = false;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
     // TODO: still under discussion whether to switch to SLS_All.
     GoogleStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
@@ -1097,7 +1085,7 @@ FormatStyle getWebKitStyle() {
   FormatStyle Style = getLLVMStyle();
   Style.AccessModifierOffset = -4;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-  Style.AlignOperands = FormatStyle::OAS_DontAlign;
+  Style.AlignOperands = false;
   Style.AlignTrailingComments = false;
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;