[PATCH] D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC

Daniel Jasper via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 5 10:40:05 PST 2018


djasper added inline comments.


================
Comment at: lib/Format/TokenAnnotator.cpp:352
+      return false;
+    if (!Tok.startsSequence(tok::l_square, tok::l_square))
+      return false;
----------------
Just join this if with the previous one.


================
Comment at: lib/Format/TokenAnnotator.cpp:357
+      return false;
+    bool NamespaceAllowed;
+    // C++17 '[[using namespace: foo, bar(baz, blech)]]'
----------------
Replace lines 355-365 with:

  bool IsUsingNamespace = AttrTok && AttrTok->startsSequence( ... );
  if (IsUsingNamespace)
    AttrTok = AttrTok->Next->Next->Next;


================
Comment at: lib/Format/TokenAnnotator.cpp:374
+      return false;
+    return AttrTok->startsSequence(tok::r_square, tok::r_square);
+  }
----------------
Why not replace these three lines with:

  return AttrTok && AttrTok->startsSequence(..);


================
Comment at: lib/Format/TokenAnnotator.cpp:400
+      next();
+      return true;
+    }
----------------
This seems weird. I think if we return true, we should have consumed everything up to the closing r_square.


================
Comment at: unittests/Format/FormatTest.cpp:12083
 
+TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
+  EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[noreturn]];"));
----------------
You are not adding any test (AFAICT) that tests that you have correctly set TT_AttributeSpecifier or that you are making any formatting decisions based on it. If you can't test it, remove that part of this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D43902





More information about the cfe-commits mailing list