r323904 - [clang-format] Align preprocessor comments with #

[Hans Wennborg via cfe-commits]

Merged in r324329.

On Wed, Jan 31, 2018 at 9:05 PM, Mark Zeren via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: mzeren-vmw
> Date: Wed Jan 31 12:05:50 2018
> New Revision: 323904
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323904&view=rev
> Log:
> [clang-format] Align preprocessor comments with #
>
> Summary:
> r312125, which introduced preprocessor indentation, shipped with a known
> issue where "indentation of comments immediately before indented
> preprocessor lines is toggled on each run". For example these two forms
> toggle:
>
>   #ifndef HEADER_H
>   #define HEADER_H
>   #if 1
>   // comment
>   #   define A 0
>   #endif
>   #endif
>
>   #ifndef HEADER_H
>   #define HEADER_H
>   #if 1
>      // comment
>   #   define A 0
>   #endif
>   #endif
>
> This happens because we check vertical alignment against the '#' yet
> indent to the level of the 'define'. This patch resolves this issue by
> aligning against the '#'.
>
> Reviewers: krasimir, klimek, djasper
>
> Reviewed By: krasimir
>
> Subscribers: cfe-commits
> Differential Revision: https://reviews.llvm.org/D42408
>
> Modified:
>     cfe/trunk/lib/Format/TokenAnnotator.cpp
>     cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=323904&r1=323903&r2=323904&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Jan 31 12:05:50 2018
> @@ -1725,15 +1725,18 @@ void TokenAnnotator::setCommentLineLevel
>        }
>      }
>
> -    if (NextNonCommentLine && CommentLine) {
> -      // If the comment is currently aligned with the line immediately following
> -      // it, that's probably intentional and we should keep it.
> -      bool AlignedWithNextLine =
> -          NextNonCommentLine->First->NewlinesBefore <= 1 &&
> -          NextNonCommentLine->First->OriginalColumn ==
> -              (*I)->First->OriginalColumn;
> -      if (AlignedWithNextLine)
> -        (*I)->Level = NextNonCommentLine->Level;
> +    // If the comment is currently aligned with the line immediately following
> +    // it, that's probably intentional and we should keep it.
> +    if (NextNonCommentLine && CommentLine &&
> +        NextNonCommentLine->First->NewlinesBefore <= 1 &&
> +        NextNonCommentLine->First->OriginalColumn ==
> +            (*I)->First->OriginalColumn) {
> +      // Align comments for preprocessor lines with the # in column 0.
> +      // Otherwise, align with the next line.
> +      (*I)->Level = (NextNonCommentLine->Type == LT_PreprocessorDirective ||
> +                     NextNonCommentLine->Type == LT_ImportStatement)
> +                        ? 0
> +                        : NextNonCommentLine->Level;
>      } else {
>        NextNonCommentLine = (*I)->First->isNot(tok::r_brace) ? (*I) : nullptr;
>      }
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=323904&r1=323903&r2=323904&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 31 12:05:50 2018
> @@ -2595,21 +2595,85 @@ TEST_F(FormatTest, IndentPreprocessorDir
>                     "code();\n"
>                     "#endif",
>                     Style));
> -  // FIXME: The comment indent corrector in TokenAnnotator gets thrown off by
> -  // preprocessor indentation.
> -  EXPECT_EQ("#if 1\n"
> -            "  // comment\n"
> -            "#  define A 0\n"
> -            "// comment\n"
> -            "#  define B 0\n"
> -            "#endif",
> -            format("#if 1\n"
> -                   "// comment\n"
> -                   "#  define A 0\n"
> -                   "   // comment\n"
> -                   "#  define B 0\n"
> -                   "#endif",
> -                   Style));
> +  // Keep comments aligned with #, otherwise indent comments normally. These
> +  // tests cannot use verifyFormat because messUp manipulates leading
> +  // whitespace.
> +  {
> +    const char *Expected = ""
> +                           "void f() {\n"
> +                           "#if 1\n"
> +                           "// Preprocessor aligned.\n"
> +                           "#  define A 0\n"
> +                           "  // Code. Separated by blank line.\n"
> +                           "\n"
> +                           "#  define B 0\n"
> +                           "  // Code. Not aligned with #\n"
> +                           "#  define C 0\n"
> +                           "#endif";
> +    const char *ToFormat = ""
> +                           "void f() {\n"
> +                           "#if 1\n"
> +                           "// Preprocessor aligned.\n"
> +                           "#  define A 0\n"
> +                           "// Code. Separated by blank line.\n"
> +                           "\n"
> +                           "#  define B 0\n"
> +                           "   // Code. Not aligned with #\n"
> +                           "#  define C 0\n"
> +                           "#endif";
> +    EXPECT_EQ(Expected, format(ToFormat, Style));
> +    EXPECT_EQ(Expected, format(Expected, Style));
> +  }
> +  // Keep block quotes aligned.
> +  {
> +    const char *Expected = ""
> +                           "void f() {\n"
> +                           "#if 1\n"
> +                           "/* Preprocessor aligned. */\n"
> +                           "#  define A 0\n"
> +                           "  /* Code. Separated by blank line. */\n"
> +                           "\n"
> +                           "#  define B 0\n"
> +                           "  /* Code. Not aligned with # */\n"
> +                           "#  define C 0\n"
> +                           "#endif";
> +    const char *ToFormat = ""
> +                           "void f() {\n"
> +                           "#if 1\n"
> +                           "/* Preprocessor aligned. */\n"
> +                           "#  define A 0\n"
> +                           "/* Code. Separated by blank line. */\n"
> +                           "\n"
> +                           "#  define B 0\n"
> +                           "   /* Code. Not aligned with # */\n"
> +                           "#  define C 0\n"
> +                           "#endif";
> +    EXPECT_EQ(Expected, format(ToFormat, Style));
> +    EXPECT_EQ(Expected, format(Expected, Style));
> +  }
> +  // Keep comments aligned with un-indented directives.
> +  {
> +    const char *Expected = ""
> +                           "void f() {\n"
> +                           "// Preprocessor aligned.\n"
> +                           "#define A 0\n"
> +                           "  // Code. Separated by blank line.\n"
> +                           "\n"
> +                           "#define B 0\n"
> +                           "  // Code. Not aligned with #\n"
> +                           "#define C 0\n";
> +    const char *ToFormat = ""
> +                           "void f() {\n"
> +                           "// Preprocessor aligned.\n"
> +                           "#define A 0\n"
> +                           "// Code. Separated by blank line.\n"
> +                           "\n"
> +                           "#define B 0\n"
> +                           "   // Code. Not aligned with #\n"
> +                           "#define C 0\n";
> +    EXPECT_EQ(Expected, format(ToFormat, Style));
> +    EXPECT_EQ(Expected, format(Expected, Style));
> +  }
>    // Test with tabs.
>    Style.UseTab = FormatStyle::UT_Always;
>    Style.IndentWidth = 8;
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits