[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion
curdeius added a comment.
Please update the summary, the name is no longer empty. Also, if you can add
some info in the summary about how it looked before (copying a part of the
Chromium bug report seems enough), it would be awesome.
Otherwise, some small comments only.
Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:59
+
+// Use the string after `namespace` until `{` or `::` or `(` as an name
+// candicate. If the name is empty, use the candicate.
Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:60
+// Use the string after `namespace` until `{` or `::` or `(` as an name
+// candicate. If the name is empty, use the candicate.
+std::string FirstNSName;
Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:69
+while (Tok && !Tok->is(tok::l_brace) && !Tok->is(tok::coloncolon) &&
+ !Tok->is(tok::l_paren)) {
+ FirstNSName += FirstNSTok->TokenText;
It should be simpler when written as `!Tok->isOneOf(...)`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120931/new/
https://reviews.llvm.org/D120931
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion
zequanwu added inline comments.
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:86
+"int j;\n"
+"}// namespace A::M",
+fixNamespaceEndComments("#define M(x) x##x\n"
MyDeveloperDay wrote:
> same here its not A::M is it? at best its A::x but it depends on the macro ?
> or have I miss understood?
Yes, you are right. Fixed to include the macro literally.
Ideally, it should expand the macro to get the name or ignore the macro.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120931/new/
https://reviews.llvm.org/D120931
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion
zequanwu updated this revision to Diff 412856.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.
Address comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120931/new/
https://reviews.llvm.org/D120931
Files:
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -68,6 +68,57 @@
"int i;\n"
"int j;\n"
"}"));
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace M(x)",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::M(x)",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace M(x)::A",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::inline M(x)::B {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::inline M(x)::B",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::inline M(x)::B {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::inline M(x)::A",
+fixNamespaceEndComments(
+"#define M(x) x##x\n"
+"namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
EXPECT_EQ("inline namespace A {\n"
"int i;\n"
"int j;\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3738,6 +3738,36 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace M(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace N::inline M(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace M(x)::inline N {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace N::M(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace M::N(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
verifyFormat("namespace N::inline D {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2597,10 +2597,12 @@
parseParens();
} else {
while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
- tok::l_square, tok::period)
[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion
MyDeveloperDay added inline comments.
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:76
+"int j;\n"
+"}// namespace",
+fixNamespaceEndComments("#define M(x) x##x\n"
but its not an anonymous namespace?
would
'''
}// namespace M(x)
'''
be clearer?
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:86
+"int j;\n"
+"}// namespace A::M",
+fixNamespaceEndComments("#define M(x) x##x\n"
same here its not A::M is it? at best its A::x but it depends on the macro ? or
have I miss understood?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120931/new/
https://reviews.llvm.org/D120931
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion
zequanwu created this revision.
zequanwu added reviewers: klimek, sammccall.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Originally filed at crbug.com/1184570.
When the name of a namespace is a macro that takes arguments,
- It fixed the indentation.
- It tried to fix namespace end comment by making the name empty, because we
don't know the macro expansion. It seems like `FormatToken::MacroCtx` is always
None. Its assignment only happens in `MacroExpander` which is never used.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120931
Files:
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -68,6 +68,27 @@
"int i;\n"
"int j;\n"
"}"));
+ // FIXME: Expand macro for namepsace name.
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+ EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::M",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
EXPECT_EQ("inline namespace A {\n"
"int i;\n"
"int j;\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3738,6 +3738,36 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace M(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace N::inline M(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace M(x)::inline N {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace N::M(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
+ verifyFormat("#define M(x) x##x\n"
+ "namespace M::N(x) {\n"
+ "class A {};\n"
+ "void f() { f(); }\n"
+ "}",
+ LLVMWithNoNamespaceFix);
verifyFormat("namespace N::inline D {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2597,10 +2597,12 @@
parseParens();
} else {
while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
- tok::l_square, tok::period) ||
+ tok::l_square, tok::period, tok::l_paren) ||
(Style.isCSharp() && FormatTok->is(tok::kw_union)))
if (FormatTok->is(tok::l_square))
parseSquare();
+ else if (FormatTok->is(tok::l_paren))
+parseParens();
else
nextToken();
}
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -51,7 +51,9 @@
}
Tok = FirstNSTok;
-while (Tok && !Tok->is(tok::l_brace)) {
+while (Tok && !Tok->is(tok::l_brace) &&
+ (Tok->Tok.isAnyIdentifier() || Tok->is(tok::coloncolon) ||
+Tok->is(tok::kw_inline))) {
name += Tok->TokenText;
if (Tok->is(tok::kw_inline))
name += "
