r311792 - [Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for

[Hans Wennborg via cfe-commits]

Merged to 5.0 in r311800.

On Fri, Aug 25, 2017 at 12:14 PM, Daniel Jasper via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: djasper
> Date: Fri Aug 25 12:14:53 2017
> New Revision: 311792
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311792&view=rev
> Log:
> [Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for
> alignments
>
> Indent should be compared before nesting level to determine if a token
> is on the same scope as the one we align with. Because it was inverted,
> clang-format sometimes tried to align tokens with tokens from outer
> scopes, causing the assert(Shift >= 0) to fire.
>
> This fixes bug #33507. Patch by Beren Minor, thank you!
>
> Modified:
>     cfe/trunk/lib/Format/WhitespaceManager.cpp
>     cfe/trunk/lib/Format/WhitespaceManager.h
>     cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=311792&r1=311791&r2=311792&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
> +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Fri Aug 25 12:14:53 2017
> @@ -246,12 +246,12 @@ AlignTokenSequence(unsigned Start, unsig
>
>    for (unsigned i = Start; i != End; ++i) {
>      if (ScopeStack.size() != 0 &&
> -        Changes[i].nestingAndIndentLevel() <
> -            Changes[ScopeStack.back()].nestingAndIndentLevel())
> +        Changes[i].indentAndNestingLevel() <
> +            Changes[ScopeStack.back()].indentAndNestingLevel())
>        ScopeStack.pop_back();
>
> -    if (i != Start && Changes[i].nestingAndIndentLevel() >
> -                          Changes[i - 1].nestingAndIndentLevel())
> +    if (i != Start && Changes[i].indentAndNestingLevel() >
> +                          Changes[i - 1].indentAndNestingLevel())
>        ScopeStack.push_back(i);
>
>      bool InsideNestedScope = ScopeStack.size() != 0;
> @@ -327,8 +327,8 @@ static unsigned AlignTokens(const Format
>
>    // Measure the scope level (i.e. depth of (), [], {}) of the first token, and
>    // abort when we hit any token in a higher scope than the starting one.
> -  auto NestingAndIndentLevel = StartAt < Changes.size()
> -                                   ? Changes[StartAt].nestingAndIndentLevel()
> +  auto IndentAndNestingLevel = StartAt < Changes.size()
> +                                   ? Changes[StartAt].indentAndNestingLevel()
>                                     : std::pair<unsigned, unsigned>(0, 0);
>
>    // Keep track of the number of commas before the matching tokens, we will only
> @@ -359,7 +359,7 @@ static unsigned AlignTokens(const Format
>
>    unsigned i = StartAt;
>    for (unsigned e = Changes.size(); i != e; ++i) {
> -    if (Changes[i].nestingAndIndentLevel() < NestingAndIndentLevel)
> +    if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
>        break;
>
>      if (Changes[i].NewlinesBefore != 0) {
> @@ -375,7 +375,7 @@ static unsigned AlignTokens(const Format
>
>      if (Changes[i].Tok->is(tok::comma)) {
>        ++CommasBeforeMatch;
> -    } else if (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel) {
> +    } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
>        // Call AlignTokens recursively, skipping over this scope block.
>        unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i);
>        i = StoppedAt - 1;
>
> Modified: cfe/trunk/lib/Format/WhitespaceManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=311792&r1=311791&r2=311792&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/WhitespaceManager.h (original)
> +++ cfe/trunk/lib/Format/WhitespaceManager.h Fri Aug 25 12:14:53 2017
> @@ -154,12 +154,11 @@ public:
>      const Change *StartOfBlockComment;
>      int IndentationOffset;
>
> -    // A combination of nesting level and indent level, which are used in
> +    // A combination of indent level and nesting level, which are used in
>      // tandem to compute lexical scope, for the purposes of deciding
>      // when to stop consecutive alignment runs.
> -    std::pair<unsigned, unsigned>
> -    nestingAndIndentLevel() const {
> -      return std::make_pair(Tok->NestingLevel, Tok->IndentLevel);
> +    std::pair<unsigned, unsigned> indentAndNestingLevel() const {
> +      return std::make_pair(Tok->IndentLevel, Tok->NestingLevel);
>      }
>    };
>
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=311792&r1=311791&r2=311792&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Aug 25 12:14:53 2017
> @@ -8958,6 +8958,16 @@ TEST_F(FormatTest, AlignConsecutiveDecla
>                 Alignment);
>    Alignment.BinPackParameters = true;
>    Alignment.ColumnLimit = 80;
> +
> +  // Bug 33507
> +  Alignment.PointerAlignment = FormatStyle::PAS_Middle;
> +  verifyFormat(
> +      "auto found = range::find_if(vsProducts, [&](auto * aProduct) {\n"
> +      "  static const Version verVs2017;\n"
> +      "  return true;\n"
> +      "});\n",
> +      Alignment);
> +  Alignment.PointerAlignment = FormatStyle::PAS_Right;
>  }
>
>  TEST_F(FormatTest, LinuxBraceBreaking) {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits