Merged in r324329.
On Wed, Jan 31, 2018 at 9:05 PM, Mark Zeren via cfe-commits <cfe-commits@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
