[PATCH] D115647: [clang-format] FixNamespaceComments does not understand namespace aliases

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 13 11:26:25 PST 2021


curdeius added inline comments.


================
Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:27
 // Returns "" for anonymous namespace.
-std::string computeName(const FormatToken *NamespaceTok) {
+bool computeName(const FormatToken *NamespaceTok, std::string &name) {
   assert(NamespaceTok &&
----------------
It seems to be a good place for optional, nope?


================
Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:183
   const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First;
   if (NamespaceTok->is(tok::l_brace)) {
     // "namespace" keyword can be on the line preceding '{', e.g. in styles
----------------
Maybe just checking previous (non-comment) token to be not a semicolon would be enough?


================
Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:250
+    std::string NamespaceName;
+    if (!computeName(NamespaceTok, NamespaceName)) {
+      // Its likely a namespace alias.
----------------
As an alternative, couldn't we avoid computing the name for namespace aliases by searching the next semicolon (don't compute) or l_brace (compute)?


================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1189
+
+TEST_F(ShortNamespaceLinesTest, NameSpaceAlias) {
+  auto Style = getLLVMStyle();
----------------
Nit: `NamespaceAlias` with small 's'.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115647/new/

https://reviews.llvm.org/D115647



More information about the cfe-commits mailing list