r297140 - [clang-format] Support namespaces ending in semicolon

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 7 08:01:45 PST 2017


Looks to be failing existing tests?

FAIL: Clang-Unit :: Format/FormatTests/FormatTest.BreaksLongDeclarations
(12427 of 32080)
******************** TEST 'Clang-Unit ::
Format/FormatTests/FormatTest.BreaksLongDeclarations' FAILED
********************
Note: Google Test filter = FormatTest.BreaksLongDeclarations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from FormatTest
[ RUN      ] FormatTest.BreaksLongDeclarations
/usr/local/google/home/blaikie/dev/llvm/src/tools/clang/unittests/Format/FormatTest.cpp:73:
Failure
      Expected: Code.str()
      Which is: "template<typename T> // Templates on own line.\nstatic int
          // Some comment.\nMyFunction(int a);"
To be equal to: format(test::messUp(Code), Style)
      Which is: "template <typename T> // Templates on own line.\nstatic
int            // Some comment.\nMyFunction(int a);"
With diff:
@@ -1,3 +1,3 @@
-template<typename T> // Templates on own line.
-static int           // Some comment.
+template <typename T> // Templates on own line.
+static int            // Some comment.
 MyFunction(int a);

/usr/local/google/home/blaikie/dev/llvm/src/tools/clang/unittests/Format/FormatTest.cpp:79:
Failure
      Expected: Code.str()
      Which is: "template<typename T> // Templates on own line.\nstatic int
          // Some comment.\nMyFunction(int a);"
To be equal to: format(test::messUp(Code), ObjCStyle)
      Which is: "template <typename T> // Templates on own line.\nstatic
int            // Some comment.\nMyFunction(int a);"
With diff:
@@ -1,3 +1,3 @@
-template<typename T> // Templates on own line.
-static int           // Some comment.
+template <typename T> // Templates on own line.
+static int            // Some comment.
 MyFunction(int a);


On Tue, Mar 7, 2017 at 6:19 AM Krasimir Georgiev via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: krasimir
> Date: Tue Mar  7 08:07:43 2017
> New Revision: 297140
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297140&view=rev
> Log:
> [clang-format] Support namespaces ending in semicolon
>
> Summary:
> This patch adds support for namespaces ending in semicolon to the
> namespace comment fixer.
> source:
> ```
> namespace A {
>   int i;
>   int j;
> };
> ```
> clang-format before:
> ```
> namespace A {
>   int i;
>   int j;
> } // namespace A;
> ```
> clang-format after:
> ```
> namespace A {
>   int i;
>   int j;
> }; // namespace A
> ```
>
> Reviewers: djasper
>
> Reviewed By: djasper
>
> Subscribers: cfe-commits, klimek
>
> Differential Revision: https://reviews.llvm.org/D30688
>
> Modified:
>     cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
>     cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
>
> Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=297140&r1=297139&r2=297140&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original)
> +++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Tue Mar  7 08:07:43
> 2017
> @@ -139,20 +139,34 @@ tooling::Replacements NamespaceEndCommen
>      if (RBraceTok->Finalized)
>        continue;
>      RBraceTok->Finalized = true;
> +    const FormatToken *EndCommentPrevTok = RBraceTok;
> +    // Namespaces often end with '};'. In that case, attach namespace end
> +    // comments to the semicolon tokens.
> +    if (RBraceTok->Next && RBraceTok->Next->is(tok::semi)) {
> +      EndCommentPrevTok = RBraceTok->Next;
> +    }
> +    // The next token in the token stream after the place where the end
> comment
> +    // token must be. This is either the next token on the current line
> or the
> +    // first token on the next line.
> +    const FormatToken *EndCommentNextTok = EndCommentPrevTok->Next;
> +    if (EndCommentNextTok && EndCommentNextTok->is(tok::comment))
> +      EndCommentNextTok = EndCommentNextTok->Next;
> +    if (!EndCommentNextTok && I + 1 < E)
> +      EndCommentNextTok = AnnotatedLines[I + 1]->First;
> +    bool AddNewline = EndCommentNextTok &&
> +                      EndCommentNextTok->NewlinesBefore == 0 &&
> +                      EndCommentNextTok->isNot(tok::eof);
>      const std::string NamespaceName = computeName(NamespaceTok);
> -    bool AddNewline = (I + 1 < E) &&
> -                      AnnotatedLines[I + 1]->First->NewlinesBefore == 0 &&
> -                      AnnotatedLines[I + 1]->First->isNot(tok::eof);
>      const std::string EndCommentText =
>          computeEndCommentText(NamespaceName, AddNewline);
> -    if (!hasEndComment(RBraceTok)) {
> +    if (!hasEndComment(EndCommentPrevTok)) {
>        bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
>        if (!isShort)
> -        addEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes);
> +        addEndComment(EndCommentPrevTok, EndCommentText, SourceMgr,
> &Fixes);
>        continue;
>      }
> -    if (!validEndComment(RBraceTok, NamespaceName))
> -      updateEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes);
> +    if (!validEndComment(EndCommentPrevTok, NamespaceName))
> +      updateEndComment(EndCommentPrevTok, EndCommentText, SourceMgr,
> &Fixes);
>    }
>    return Fixes;
>  }
>
> Modified: cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp?rev=297140&r1=297139&r2=297140&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp (original)
> +++ cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp Tue Mar
> 7 08:07:43 2017
> @@ -184,6 +184,34 @@ TEST_F(NamespaceEndCommentsFixerTest, Ad
>                                      "}\n"
>                                      "}\n"
>                                      "}"));
> +
> +  // Adds an end comment after a semicolon.
> +  EXPECT_EQ("namespace {\n"
> +            "  int i;\n"
> +            "  int j;\n"
> +            "};// namespace",
> +            fixNamespaceEndComments("namespace {\n"
> +                                    "  int i;\n"
> +                                    "  int j;\n"
> +                                    "};"));
> +  EXPECT_EQ("namespace A {\n"
> +            "  int i;\n"
> +            "  int j;\n"
> +            "};// namespace A",
> +            fixNamespaceEndComments("namespace A {\n"
> +                                    "  int i;\n"
> +                                    "  int j;\n"
> +                                    "};"));
> +  EXPECT_EQ("namespace A {\n"
> +            "  int i;\n"
> +            "  int j;\n"
> +            "};// namespace A\n"
> +            "// unrelated",
> +            fixNamespaceEndComments("namespace A {\n"
> +                                    "  int i;\n"
> +                                    "  int j;\n"
> +                                    "};\n"
> +                                    "// unrelated"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
> @@ -220,6 +248,24 @@ TEST_F(NamespaceEndCommentsFixerTest, Ad
>                                      "  int j;\n"
>                                      "  int k;\n"
>                                      "}"));
> +  EXPECT_EQ("namespace {\n"
> +            "  int i;\n"
> +            "  int j;\n"
> +            "};// namespace\n"
> +            "int k;",
> +            fixNamespaceEndComments("namespace {\n"
> +                                    "  int i;\n"
> +                                    "  int j;\n"
> +                                    "};int k;"));
> +  EXPECT_EQ("namespace {\n"
> +            "  int i;\n"
> +            "  int j;\n"
> +            "};// namespace\n"
> +            ";",
> +            fixNamespaceEndComments("namespace {\n"
> +                                    "  int i;\n"
> +                                    "  int j;\n"
> +                                    "};;"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest,
> DoesNotAddEndCommentForShortNamespace) {
> @@ -227,6 +273,8 @@ TEST_F(NamespaceEndCommentsFixerTest, Do
>    EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
>    EXPECT_EQ("namespace A { a }",
>              fixNamespaceEndComments("namespace A { a }"));
> +  EXPECT_EQ("namespace A { a };",
> +            fixNamespaceEndComments("namespace A { a };"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest,
> DoesNotAddCommentAfterUnaffectedRBrace) {
> @@ -238,6 +286,14 @@ TEST_F(NamespaceEndCommentsFixerTest, Do
>                                      "}",
>                                      // The range (16, 3) spans the 'int'
> above.
>                                      /*Ranges=*/{1, tooling::Range(16,
> 3)}));
> +  EXPECT_EQ("namespace A {\n"
> +            "  int i;\n"
> +            "};",
> +            fixNamespaceEndComments("namespace A {\n"
> +                                    "  int i;\n"
> +                                    "};",
> +                                    // The range (16, 3) spans the 'int'
> above.
> +                                    /*Ranges=*/{1, tooling::Range(16,
> 3)}));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest,
> DoesNotAddCommentAfterRBraceInPPDirective) {
> @@ -276,6 +332,18 @@ TEST_F(NamespaceEndCommentsFixerTest, Ke
>              fixNamespaceEndComments("namespace A::B {\n"
>                                      "  int i;\n"
>                                      "} // end namespace A::B"));
> +  EXPECT_EQ("namespace A {\n"
> +            "  int i;\n"
> +            "}; // end namespace A",
> +            fixNamespaceEndComments("namespace A {\n"
> +                                    "  int i;\n"
> +                                    "}; // end namespace A"));
> +  EXPECT_EQ("namespace {\n"
> +            "  int i;\n"
> +            "}; /* unnamed namespace */",
> +            fixNamespaceEndComments("namespace {\n"
> +                                    "  int i;\n"
> +                                    "}; /* unnamed namespace */"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) {
> @@ -309,10 +377,17 @@ TEST_F(NamespaceEndCommentsFixerTest, Up
>              fixNamespaceEndComments("namespace A {\n"
>                                      "  int i;\n"
>                                      "} // banamespace A"));
> -
> +  EXPECT_EQ("namespace A {\n"
> +            "  int i;\n"
> +            "}; // namespace A",
> +            fixNamespaceEndComments("namespace A {\n"
> +                                    "  int i;\n"
> +                                    "}; // banamespace A"));
>    // Updates invalid line comments even for short namespaces.
>    EXPECT_EQ("namespace A {} // namespace A",
>              fixNamespaceEndComments("namespace A {} // namespace"));
> +  EXPECT_EQ("namespace A {}; // namespace A",
> +            fixNamespaceEndComments("namespace A {}; // namespace"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndBlockComment) {
> @@ -346,8 +421,16 @@ TEST_F(NamespaceEndCommentsFixerTest, Up
>              fixNamespaceEndComments("namespace A {\n"
>                                      "  int i;\n"
>                                      "} /* banamespace A */"));
> +  EXPECT_EQ("namespace A {\n"
> +            "  int i;\n"
> +            "}; // namespace A",
> +            fixNamespaceEndComments("namespace A {\n"
> +                                    "  int i;\n"
> +                                    "}; /* banamespace A */"));
>    EXPECT_EQ("namespace A {} // namespace A",
>              fixNamespaceEndComments("namespace A {} /**/"));
> +  EXPECT_EQ("namespace A {}; // namespace A",
> +            fixNamespaceEndComments("namespace A {}; /**/"));
>  }
>
>  TEST_F(NamespaceEndCommentsFixerTest,
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170307/e0990329/attachment-0001.html>


More information about the cfe-commits mailing list