[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver added a comment. In https://reviews.llvm.org/D51036#1223254, @sammccall wrote: > In https://reviews.llvm.org/D51036#1223230, @melver wrote: > > > Awaiting remaining reviewer acceptance. > > > > FYI: I do not have commit commit access -- what is the procedure to commit > > once diff is accepted? > > > > Many thanks! > > > Anyone with commit access can land it for you - I'm happy to do this. > @owenpan any concerns? Great, many thanks for committing. Repository: rL LLVM https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341450: clang-format: Fix formatting C++ namespaces with
preceding inline or export… (authored by sammccall,
committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D51036
Files:
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
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -992,13 +992,6 @@
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 @@
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)) {
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1309,8 +1309,7 @@
std::set DeletedLines;
for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
auto = *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 @@
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;
Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -125,12 +125,7 @@
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 ,
Index: cfe/trunk/lib/Format/TokenAnnotator.h
===
--- cfe/trunk/lib/Format/TokenAnnotator.h
+++ cfe/trunk/lib/Format/TokenAnnotator.h
@@ -105,6 +105,13 @@
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;
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -535,7 +535,7 @@
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 @@
// 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;
Index:
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D51036#1223254, @sammccall wrote: > In https://reviews.llvm.org/D51036#1223230, @melver wrote: > > > Awaiting remaining reviewer acceptance. > > > > FYI: I do not have commit commit access -- what is the procedure to commit > > once diff is accepted? > > > > Many thanks! > > > Anyone with commit access can land it for you - I'm happy to do this. > @owenpan any concerns? @sammccall Go ahead! :) Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
sammccall added a comment. In https://reviews.llvm.org/D51036#1223230, @melver wrote: > Awaiting remaining reviewer acceptance. > > FYI: I do not have commit commit access -- what is the procedure to commit > once diff is accepted? > > Many thanks! Anyone with commit access can land it for you - I'm happy to do this. @owenpan any concerns? Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver added a comment. Awaiting remaining reviewer acceptance. FYI: I do not have commit commit access -- what is the procedure to commit once diff is accepted? Many thanks! Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver added inline comments.
Comment at: lib/Format/Format.cpp:1312
auto = *AnnotatedLines[i];
if (Line.startsWith(tok::kw_namespace) ||
+ Line.startsWith(tok::kw_inline, tok::kw_namespace) ||
owenpan wrote:
> sammccall wrote:
> > these 3 startswith checks appear 4 times now, you could pull out a helper
> > function `startsWithNamespace` in FormatToken.h or something like that.
> > Up to you.
> I missed it. Good catch!
Added startsWithNamespace to TokenAnnotator.h.
Comment at: unittests/Format/FormatTest.cpp:7582
Style);
+ verifyFormat("export namespace Foo\n"
+ "{};",
sammccall wrote:
> you may want to add tests for other modules TS syntax (e.g. non-namespace
> export decls).
> It seems this works well today, but without tests it could regress.
I've added a couple for 'export class' with access specifiers. Otherwise, any
other additions should probably be in future non-namespace related patches.
Repository:
rC Clang
https://reviews.llvm.org/D51036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver updated this revision to Diff 162653.
melver marked 4 inline comments as done.
melver added a comment.
Many thanks! PTAL.
Repository:
rC Clang
https://reviews.llvm.org/D51036
Files:
lib/Format/Format.cpp
lib/Format/FormatToken.h
lib/Format/NamespaceEndCommentsFixer.cpp
lib/Format/TokenAnnotator.h
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -200,6 +200,42 @@
"inti;\n"
"}",
getGoogleStyle()));
+ EXPECT_EQ("/* something */ namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("/* something */ namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
+ EXPECT_EQ("inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("inline namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
+ EXPECT_EQ("/* something */ inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("/* something */ inline namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
+ EXPECT_EQ("export namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("export namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
EXPECT_EQ("extern /**/ \"C\" /**/ {\n"
"\n"
"int i;\n"
@@ -1186,12 +1222,25 @@
"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"
@@ -1563,16 +1612,36 @@
"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(); }",
@@ -7556,6 +7625,12 @@
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"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -989,13 +989,6 @@
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
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan added inline comments. Comment at: lib/Format/UnwrappedLineFormatter.cpp:533-535 + !(Line.startsWith(tok::kw_namespace) || + Line.startsWith(tok::kw_inline, tok::kw_namespace) || + Line.startsWith(tok::kw_export, tok::kw_namespace)) && Maybe add a test case (or modify an existing one) for this fix, with a C/C++ style comment before `namespace`, `inline`, and/or `export`? Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan added inline comments. Comment at: lib/Format/Format.cpp:1312 auto = *AnnotatedLines[i]; if (Line.startsWith(tok::kw_namespace) || + Line.startsWith(tok::kw_inline, tok::kw_namespace) || sammccall wrote: > these 3 startswith checks appear 4 times now, you could pull out a helper > function `startsWithNamespace` in FormatToken.h or something like that. > Up to you. I missed it. Good catch! Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
sammccall added inline comments.
Comment at: unittests/Format/FormatTest.cpp:7582
Style);
+ verifyFormat("export namespace Foo\n"
+ "{};",
you may want to add tests for other modules TS syntax (e.g. non-namespace
export decls).
It seems this works well today, but without tests it could regress.
Repository:
rC Clang
https://reviews.llvm.org/D51036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
sammccall accepted this revision. sammccall added a comment. Nice fix! Comment at: lib/Format/Format.cpp:1312 auto = *AnnotatedLines[i]; if (Line.startsWith(tok::kw_namespace) || + Line.startsWith(tok::kw_inline, tok::kw_namespace) || these 3 startswith checks appear 4 times now, you could pull out a helper function `startsWithNamespace` in FormatToken.h or something like that. Up to you. Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver updated this revision to Diff 161960.
melver marked 8 inline comments as done.
melver added a comment.
Many thanks for the suggestions!
PTAL.
Repository:
rC Clang
https://reviews.llvm.org/D51036
Files:
lib/Format/Format.cpp
lib/Format/FormatToken.h
lib/Format/NamespaceEndCommentsFixer.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -200,6 +200,24 @@
"inti;\n"
"}",
getGoogleStyle()));
+ EXPECT_EQ("inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("inline namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
+ EXPECT_EQ("export namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("export namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
EXPECT_EQ("extern /**/ \"C\" /**/ {\n"
"\n"
"int i;\n"
@@ -1573,6 +1591,11 @@
"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(); }",
@@ -7556,6 +7579,9 @@
verifyFormat("inline namespace Foo\n"
"{};",
Style);
+ verifyFormat("export namespace Foo\n"
+ "{};",
+ Style);
verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -989,13 +989,6 @@
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:
@@ -1063,6 +1056,16 @@
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)) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -529,7 +529,10 @@
Tok->SpacesRequiredBefore = 0;
Tok->CanBreakBefore = true;
return 1;
- } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) &&
+ } else if (Limit != 0 &&
+ !(Line.startsWith(tok::kw_namespace) ||
+ Line.startsWith(tok::kw_inline, tok::kw_namespace) ||
+ Line.startsWith(tok::kw_export, tok::kw_namespace)) &&
!startsExternCBlock(Line)) {
// We don't merge short records.
FormatToken *RecordTok = Line.First;
@@ -1154,7 +1157,9 @@
// Remove empty lines after "{".
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
PreviousLine->Last->is(tok::l_brace) &&
- PreviousLine->First->isNot(tok::kw_namespace) &&
+ !(PreviousLine->startsWith(tok::kw_namespace) ||
+PreviousLine->startsWith(tok::kw_inline, tok::kw_namespace) ||
+PreviousLine->startsWith(tok::kw_export, tok::kw_namespace)) &&
!startsExternCBlock(*PreviousLine))
Newlines = 1;
Index: lib/Format/NamespaceEndCommentsFixer.cpp
===
--- lib/Format/NamespaceEndCommentsFixer.cpp
+++ lib/Format/NamespaceEndCommentsFixer.cpp
@@ -125,12 +125,7 @@
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 ,
Index: lib/Format/FormatToken.h
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:1066-1072
+if (Style.isCpp()) {
+ nextToken();
+ if (FormatTok->Tok.is(tok::kw_namespace)) {
+parseNamespace();
+return;
+ }
+}
owenpan wrote:
> ```
> if (!Style.isCpp())
> break;
> case tok::kw_inline:
> nextToken();
> if (FormatTok->Tok.is(tok::kw_namespace)) {
> parseNamespace();
> return;
> }
> ```
I forgot to include LLVM_FALLTHROUGH to suppress the warning:
```
if (!Style.isCpp())
break;
LLVM_FALLTHROUGH;
case tok::kw_inline:
nextToken();
if (FormatTok->Tok.is(tok::kw_namespace)) {
parseNamespace();
return;
}
```
Repository:
rC Clang
https://reviews.llvm.org/D51036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan added inline comments. Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:128-133 + // Detect "(inline|export)? namespace" in the beginning of a line. + if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export)) NamespaceTok = NamespaceTok->getNextNonComment(); if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace)) return nullptr; return NamespaceTok; owenpan wrote: > These lines are functionally the same as lines 523-528 in FormatToken.h. > Refactor them? I think this is better: ``` return NamespaceTok->getNamespaceToken(); ``` Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan added inline comments. Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:129-130 + // Detect "(inline|export)? namespace" in the beginning of a line. + if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export)) NamespaceTok = NamespaceTok->getNextNonComment(); if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace)) owenpan wrote: > ``` > if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export)) > ``` Actually, I meant it for line 129 only. Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.
By the way, I didn't review the test cases.
Comment at: lib/Format/FormatToken.h:524-525
+// Detect "(inline|export)? namespace" in the beginning of a line.
+if (NamespaceTok &&
+(NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export)))
NamespaceTok = NamespaceTok->getNextNonComment();
```
if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export))
```
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:128-133
+ // Detect "(inline|export)? namespace" in the beginning of a line.
+ if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
NamespaceTok = NamespaceTok->getNextNonComment();
if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
return nullptr;
return NamespaceTok;
These lines are functionally the same as lines 523-528 in FormatToken.h.
Refactor them?
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:129-130
+ // Detect "(inline|export)? namespace" in the beginning of a line.
+ if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
NamespaceTok = NamespaceTok->getNextNonComment();
if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
```
if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export))
```
Comment at: lib/Format/UnwrappedLineParser.cpp:992-998
case tok::kw_inline:
nextToken();
if (FormatTok->Tok.is(tok::kw_namespace)) {
parseNamespace();
return;
}
break;
Move this case to after the case tok::kw_export below.
Comment at: lib/Format/UnwrappedLineParser.cpp:1066-1072
+if (Style.isCpp()) {
+ nextToken();
+ if (FormatTok->Tok.is(tok::kw_namespace)) {
+parseNamespace();
+return;
+ }
+}
```
if (!Style.isCpp())
break;
case tok::kw_inline:
nextToken();
if (FormatTok->Tok.is(tok::kw_namespace)) {
parseNamespace();
return;
}
```
Repository:
rC Clang
https://reviews.llvm.org/D51036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver created this revision.
Herald added a subscriber: cfe-commits.
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.
Repository:
rC Clang
https://reviews.llvm.org/D51036
Files:
lib/Format/Format.cpp
lib/Format/FormatToken.h
lib/Format/NamespaceEndCommentsFixer.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -200,6 +200,24 @@
"inti;\n"
"}",
getGoogleStyle()));
+ EXPECT_EQ("inline namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("inline namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
+ EXPECT_EQ("export namespace N {\n"
+"\n"
+"int i;\n"
+"}",
+format("export namespace N {\n"
+ "\n"
+ "inti;\n"
+ "}",
+ getGoogleStyle()));
EXPECT_EQ("extern /**/ \"C\" /**/ {\n"
"\n"
"int i;\n"
@@ -1573,6 +1591,11 @@
"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(); }",
@@ -7556,6 +7579,9 @@
verifyFormat("inline namespace Foo\n"
"{};",
Style);
+ verifyFormat("export namespace Foo\n"
+ "{};",
+ Style);
verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1063,6 +1063,13 @@
parseJavaScriptEs6ImportExport();
return;
}
+if (Style.isCpp()) {
+ nextToken();
+ if (FormatTok->Tok.is(tok::kw_namespace)) {
+parseNamespace();
+return;
+ }
+}
break;
case tok::identifier:
if (FormatTok->is(TT_ForEachMacro)) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -529,7 +529,10 @@
Tok->SpacesRequiredBefore = 0;
Tok->CanBreakBefore = true;
return 1;
- } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) &&
+ } else if (Limit != 0 &&
+ !(Line.startsWith(tok::kw_namespace) ||
+ Line.startsWith(tok::kw_inline, tok::kw_namespace) ||
+ Line.startsWith(tok::kw_export, tok::kw_namespace)) &&
!startsExternCBlock(Line)) {
// We don't merge short records.
FormatToken *RecordTok = Line.First;
@@ -1154,7 +1157,9 @@
// Remove empty lines after "{".
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
PreviousLine->Last->is(tok::l_brace) &&
- PreviousLine->First->isNot(tok::kw_namespace) &&
+ !(PreviousLine->startsWith(tok::kw_namespace) ||
+PreviousLine->startsWith(tok::kw_inline, tok::kw_namespace) ||
+PreviousLine->startsWith(tok::kw_export, tok::kw_namespace)) &&
!startsExternCBlock(*PreviousLine))
Newlines = 1;
Index: lib/Format/NamespaceEndCommentsFixer.cpp
===
--- lib/Format/NamespaceEndCommentsFixer.cpp
+++ lib/Format/NamespaceEndCommentsFixer.cpp
@@ -125,8 +125,8 @@
if (StartLineIndex > 0)
NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First;
}
- // Detect "(inline)? namespace" in the beginning of a line.
- if (NamespaceTok->is(tok::kw_inline))
+ // Detect "(inline|export)? namespace" in the beginning of a line.
+ if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
NamespaceTok = NamespaceTok->getNextNonComment();
if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
return nullptr;
Index: lib/Format/FormatToken.h
