r341450 - clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 5 00:44:02 PDT 2018


Author: sammccall
Date: Wed Sep  5 00:44:02 2018
New Revision: 341450

URL: http://llvm.org/viewvc/llvm-project?rev=341450&view=rev
Log:
clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

This fixes formatting namespaces with preceding 'inline' and 'export' (Modules TS) specifiers.

This change fixes namespaces not being identified as such with preceding 'inline' or 'export' specifiers.

Motivation: I was experimenting with the Modules TS (-fmodules-ts) and found it would be useful if clang-format would correctly format 'export namespace'. While making the changes, I noticed that similar issues still exist with 'inline namespace', and addressed them as well.

Patch by Marco Elver!

Reviewers: klimek, djasper, owenpan, sammccall

Reviewed By: owenpan, sammccall

Subscribers: owenpan, cfe-commits

Differential Revision: https://reviews.llvm.org/D51036

Modified:
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/lib/Format/FormatToken.h
    cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
    cfe/trunk/lib/Format/TokenAnnotator.h
    cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
    cfe/trunk/lib/Format/UnwrappedLineParser.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Sep  5 00:44:02 2018
@@ -1309,8 +1309,7 @@ private:
     std::set<unsigned> DeletedLines;
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       auto &Line = *AnnotatedLines[i];
-      if (Line.startsWith(tok::kw_namespace) ||
-          Line.startsWith(tok::kw_inline, tok::kw_namespace)) {
+      if (Line.startsWithNamespace()) {
         checkEmptyNamespace(AnnotatedLines, i, i, DeletedLines);
       }
     }
@@ -1347,9 +1346,7 @@ private:
       if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace))
         break;
 
-      if (AnnotatedLines[CurrentLine]->startsWith(tok::kw_namespace) ||
-          AnnotatedLines[CurrentLine]->startsWith(tok::kw_inline,
-                                                  tok::kw_namespace)) {
+      if (AnnotatedLines[CurrentLine]->startsWithNamespace()) {
         if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine,
                                  DeletedLines))
           return false;

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Wed Sep  5 00:44:02 2018
@@ -520,8 +520,8 @@ struct FormatToken {
     const FormatToken *NamespaceTok = this;
     if (is(tok::comment))
       NamespaceTok = NamespaceTok->getNextNonComment();
-    // Detect "(inline)? namespace" in the beginning of a line.
-    if (NamespaceTok && NamespaceTok->is(tok::kw_inline))
+    // Detect "(inline|export)? namespace" in the beginning of a line.
+    if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export))
       NamespaceTok = NamespaceTok->getNextNonComment();
     return NamespaceTok && NamespaceTok->is(tok::kw_namespace) ? NamespaceTok
                                                                : nullptr;

Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original)
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Wed Sep  5 00:44:02 2018
@@ -125,12 +125,7 @@ getNamespaceToken(const AnnotatedLine *L
     if (StartLineIndex > 0)
       NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First;
   }
-  // Detect "(inline)? namespace" in the beginning of a line.
-  if (NamespaceTok->is(tok::kw_inline))
-    NamespaceTok = NamespaceTok->getNextNonComment();
-  if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
-    return nullptr;
-  return NamespaceTok;
+  return NamespaceTok->getNamespaceToken();
 }
 
 NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env,

Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Wed Sep  5 00:44:02 2018
@@ -105,6 +105,13 @@ public:
     return !Last->isOneOf(tok::semi, tok::comment);
   }
 
+  /// \c true if this line starts a namespace definition.
+  bool startsWithNamespace() const {
+    return startsWith(tok::kw_namespace) ||
+           startsWith(tok::kw_inline, tok::kw_namespace) ||
+           startsWith(tok::kw_export, tok::kw_namespace);
+  }
+
   FormatToken *First;
   FormatToken *Last;
 

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Wed Sep  5 00:44:02 2018
@@ -535,7 +535,7 @@ private:
         Tok->SpacesRequiredBefore = 0;
         Tok->CanBreakBefore = true;
         return 1;
-      } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) &&
+      } else if (Limit != 0 && !Line.startsWithNamespace() &&
                  !startsExternCBlock(Line)) {
         // We don't merge short records.
         FormatToken *RecordTok = Line.First;
@@ -1160,7 +1160,7 @@ void UnwrappedLineFormatter::formatFirst
   // Remove empty lines after "{".
   if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
       PreviousLine->Last->is(tok::l_brace) &&
-      PreviousLine->First->isNot(tok::kw_namespace) &&
+      !PreviousLine->startsWithNamespace() &&
       !startsExternCBlock(*PreviousLine))
     Newlines = 1;
 

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed Sep  5 00:44:02 2018
@@ -992,13 +992,6 @@ void UnwrappedLineParser::parseStructura
   case tok::kw_namespace:
     parseNamespace();
     return;
-  case tok::kw_inline:
-    nextToken();
-    if (FormatTok->Tok.is(tok::kw_namespace)) {
-      parseNamespace();
-      return;
-    }
-    break;
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
@@ -1066,6 +1059,16 @@ void UnwrappedLineParser::parseStructura
       parseJavaScriptEs6ImportExport();
       return;
     }
+    if (!Style.isCpp())
+      break;
+    // Handle C++ "(inline|export) namespace".
+    LLVM_FALLTHROUGH;
+  case tok::kw_inline:
+    nextToken();
+    if (FormatTok->Tok.is(tok::kw_namespace)) {
+      parseNamespace();
+      return;
+    }
     break;
   case tok::identifier:
     if (FormatTok->is(TT_ForEachMacro)) {

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=341450&r1=341449&r2=341450&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Sep  5 00:44:02 2018
@@ -200,6 +200,42 @@ TEST_F(FormatTest, RemovesEmptyLines) {
                    "int    i;\n"
                    "}",
                    getGoogleStyle()));
+  EXPECT_EQ("/* something */ namespace N {\n"
+            "\n"
+            "int i;\n"
+            "}",
+            format("/* something */ namespace N {\n"
+                   "\n"
+                   "int    i;\n"
+                   "}",
+                   getGoogleStyle()));
+  EXPECT_EQ("inline namespace N {\n"
+            "\n"
+            "int i;\n"
+            "}",
+            format("inline namespace N {\n"
+                   "\n"
+                   "int    i;\n"
+                   "}",
+                   getGoogleStyle()));
+  EXPECT_EQ("/* something */ inline namespace N {\n"
+            "\n"
+            "int i;\n"
+            "}",
+            format("/* something */ inline namespace N {\n"
+                   "\n"
+                   "int    i;\n"
+                   "}",
+                   getGoogleStyle()));
+  EXPECT_EQ("export namespace N {\n"
+            "\n"
+            "int i;\n"
+            "}",
+            format("export namespace N {\n"
+                   "\n"
+                   "int    i;\n"
+                   "}",
+                   getGoogleStyle()));
   EXPECT_EQ("extern /**/ \"C\" /**/ {\n"
             "\n"
             "int i;\n"
@@ -1220,12 +1256,25 @@ TEST_F(FormatTest, UnderstandsAccessSpec
                "private:\n"
                "  void f() {}\n"
                "};");
+  verifyFormat("export class A {\n"
+               "public:\n"
+               "public: // comment\n"
+               "protected:\n"
+               "private:\n"
+               "  void f() {}\n"
+               "};");
   verifyGoogleFormat("class A {\n"
                      " public:\n"
                      " protected:\n"
                      " private:\n"
                      "  void f() {}\n"
                      "};");
+  verifyGoogleFormat("export class A {\n"
+                     " public:\n"
+                     " protected:\n"
+                     " private:\n"
+                     "  void f() {}\n"
+                     "};");
   verifyFormat("class A {\n"
                "public slots:\n"
                "  void f1() {}\n"
@@ -1597,16 +1646,36 @@ TEST_F(FormatTest, FormatsNamespaces) {
                "void f() { f(); }\n"
                "}",
                LLVMWithNoNamespaceFix);
+  verifyFormat("/* something */ namespace some_namespace {\n"
+               "class A {};\n"
+               "void f() { f(); }\n"
+               "}",
+               LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
                "class A {};\n"
                "void f() { f(); }\n"
                "}",
                LLVMWithNoNamespaceFix);
+  verifyFormat("/* something */ namespace {\n"
+               "class A {};\n"
+               "void f() { f(); }\n"
+               "}",
+               LLVMWithNoNamespaceFix);
   verifyFormat("inline namespace X {\n"
                "class A {};\n"
                "void f() { f(); }\n"
                "}",
                LLVMWithNoNamespaceFix);
+  verifyFormat("/* something */ inline namespace X {\n"
+               "class A {};\n"
+               "void f() { f(); }\n"
+               "}",
+               LLVMWithNoNamespaceFix);
+  verifyFormat("export namespace X {\n"
+               "class A {};\n"
+               "void f() { f(); }\n"
+               "}",
+               LLVMWithNoNamespaceFix);
   verifyFormat("using namespace some_namespace;\n"
                "class A {};\n"
                "void f() { f(); }",
@@ -7602,6 +7671,12 @@ TEST_F(FormatTest, SplitEmptyNamespace)
   verifyFormat("inline namespace Foo\n"
                "{};",
                Style);
+  verifyFormat("/* something */ inline namespace Foo\n"
+               "{};",
+               Style);
+  verifyFormat("export namespace Foo\n"
+               "{};",
+               Style);
   verifyFormat("namespace Foo\n"
                "{\n"
                "void Bar();\n"




More information about the cfe-commits mailing list