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

Ben Hamilton via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 12 08:42:38 PDT 2018


Author: benhamilton
Date: Mon Mar 12 08:42:38 2018
New Revision: 327284

URL: http://llvm.org/viewvc/llvm-project?rev=327284&view=rev
Log:
[clang-format] Don't detect C++11 attribute specifiers as ObjC

Summary:
Previously, clang-format would detect C++11 and C++17 attribute
specifiers like the following as Objective-C method invocations:

  [[noreturn]];
  [[clang::fallthrough]];
  [[noreturn, deprecated("so sorry")]];
  [[using gsl: suppress("type")]];

To fix this, I ported part of the logic from
tools/clang/lib/Parse/ParseTentative.cpp into TokenAnnotator.cpp so we
can explicitly parse and identify C++11 attribute specifiers.

This allows the guessLanguage() and getStyle() APIs to correctly
guess files containing the C++11 attribute specifiers as C++,
not Objective-C.

Test Plan: New tests added. Ran tests with:
  make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: krasimir, jolesiak, djasper

Reviewed By: djasper

Subscribers: aaron.ballman, cfe-commits, klimek

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

Modified:
    cfe/trunk/lib/Format/ContinuationIndenter.cpp
    cfe/trunk/lib/Format/FormatToken.h
    cfe/trunk/lib/Format/TokenAnnotator.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=327284&r1=327283&r2=327284&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Mar 12 08:42:38 2018
@@ -889,8 +889,8 @@ unsigned ContinuationIndenter::getNewLin
   if ((PreviousNonComment &&
        (PreviousNonComment->ClosesTemplateDeclaration ||
         PreviousNonComment->isOneOf(
-            TT_AttributeParen, TT_FunctionAnnotationRParen, TT_JavaAnnotation,
-            TT_LeadingJavaAnnotation))) ||
+            TT_AttributeParen, TT_AttributeSquare, TT_FunctionAnnotationRParen,
+            TT_JavaAnnotation, TT_LeadingJavaAnnotation))) ||
       (!Style.IndentWrappedFunctionNames &&
        NextNonComment->isOneOf(tok::kw_operator, TT_FunctionDeclarationName)))
     return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=327284&r1=327283&r2=327284&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Mon Mar 12 08:42:38 2018
@@ -29,7 +29,9 @@ namespace format {
 #define LIST_TOKEN_TYPES                                                       \
   TYPE(ArrayInitializerLSquare)                                                \
   TYPE(ArraySubscriptLSquare)                                                  \
+  TYPE(AttributeColon)                                                         \
   TYPE(AttributeParen)                                                         \
+  TYPE(AttributeSquare)                                                        \
   TYPE(BinaryOperator)                                                         \
   TYPE(BitFieldColon)                                                          \
   TYPE(BlockComment)                                                           \

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=327284&r1=327283&r2=327284&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Mar 12 08:42:38 2018
@@ -328,13 +328,40 @@ private:
     return false;
   }
 
+  bool isCpp11AttributeSpecifier(const FormatToken &Tok) {
+    if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
+      return false;
+    const FormatToken *AttrTok = Tok.Next->Next;
+    if (!AttrTok)
+      return false;
+    // C++17 '[[using ns: foo, bar(baz, blech)]]'
+    // We assume nobody will name an ObjC variable 'using'.
+    if (AttrTok->startsSequence(tok::kw_using, tok::identifier, tok::colon))
+      return true;
+    if (AttrTok->isNot(tok::identifier))
+      return false;
+    while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
+      // ObjC message send. We assume nobody will use : in a C++11 attribute
+      // specifier parameter, although this is technically valid:
+      // [[foo(:)]]
+      if (AttrTok->is(tok::colon) ||
+          AttrTok->startsSequence(tok::identifier, tok::identifier))
+        return false;
+      if (AttrTok->is(tok::ellipsis))
+        return true;
+      AttrTok = AttrTok->Next;
+    }
+    return AttrTok && AttrTok->startsSequence(tok::r_square, tok::r_square);
+  }
+
   bool parseSquare() {
     if (!CurrentToken)
       return false;
 
     // A '[' could be an index subscript (after an identifier or after
     // ')' or ']'), it could be the start of an Objective-C method
-    // expression, or it could the start of an Objective-C array literal.
+    // expression, it could the start of an Objective-C array literal,
+    // or it could be a C++ attribute specifier [[foo::bar]].
     FormatToken *Left = CurrentToken->Previous;
     Left->ParentBracket = Contexts.back().ContextKind;
     FormatToken *Parent = Left->getPreviousNonComment();
@@ -347,8 +374,11 @@ private:
         (Contexts.back().CanBeExpression || Contexts.back().IsExpression ||
          Contexts.back().InTemplateArgument);
 
+    bool IsCpp11AttributeSpecifier = isCpp11AttributeSpecifier(*Left) ||
+                                     Contexts.back().InCpp11AttributeSpecifier;
+
     bool StartsObjCMethodExpr =
-        !CppArrayTemplates && Style.isCpp() &&
+        !CppArrayTemplates && Style.isCpp() && !IsCpp11AttributeSpecifier &&
         Contexts.back().CanBeExpression && Left->isNot(TT_LambdaLSquare) &&
         CurrentToken->isNot(tok::l_brace) &&
         (!Parent ||
@@ -365,6 +395,8 @@ private:
     } else if (Left->is(TT_Unknown)) {
       if (StartsObjCMethodExpr) {
         Left->Type = TT_ObjCMethodExpr;
+      } else if (IsCpp11AttributeSpecifier) {
+        Left->Type = TT_AttributeSquare;
       } else if (Style.Language == FormatStyle::LK_JavaScript && Parent &&
                  Contexts.back().ContextKind == tok::l_brace &&
                  Parent->isOneOf(tok::l_brace, tok::comma)) {
@@ -432,11 +464,14 @@ private:
       Contexts.back().IsExpression = false;
 
     Contexts.back().ColonIsObjCMethodExpr = StartsObjCMethodExpr;
+    Contexts.back().InCpp11AttributeSpecifier = IsCpp11AttributeSpecifier;
 
     while (CurrentToken) {
       if (CurrentToken->is(tok::r_square)) {
-        if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
-            Left->is(TT_ObjCMethodExpr)) {
+        if (IsCpp11AttributeSpecifier)
+          CurrentToken->Type = TT_AttributeSquare;
+        else if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
+                 Left->is(TT_ObjCMethodExpr)) {
           // An ObjC method call is rarely followed by an open parenthesis.
           // FIXME: Do we incorrectly label ":" with this?
           StartsObjCMethodExpr = false;
@@ -466,8 +501,14 @@ private:
       if (CurrentToken->isOneOf(tok::r_paren, tok::r_brace))
         return false;
       if (CurrentToken->is(tok::colon)) {
-        if (Left->isOneOf(TT_ArraySubscriptLSquare,
-                          TT_DesignatedInitializerLSquare)) {
+        if (IsCpp11AttributeSpecifier &&
+            CurrentToken->endsSequence(tok::colon, tok::identifier,
+                                       tok::kw_using)) {
+          // Remember that this is a [[using ns: foo]] C++ attribute, so we
+          // don't add a space before the colon (unlike other colons).
+          CurrentToken->Type = TT_AttributeColon;
+        } else if (Left->isOneOf(TT_ArraySubscriptLSquare,
+                                 TT_DesignatedInitializerLSquare)) {
           Left->Type = TT_ObjCMethodExpr;
           StartsObjCMethodExpr = true;
           // ParameterCount might have been set to 1 before expression was
@@ -1088,6 +1129,7 @@ private:
     bool InInheritanceList = false;
     bool CaretFound = false;
     bool IsForEachMacro = false;
+    bool InCpp11AttributeSpecifier = false;
   };
 
   /// \brief Puts a new \c Context onto the stack \c Contexts for the lifetime
@@ -2124,7 +2166,7 @@ unsigned TokenAnnotator::splitPenalty(co
       return 35;
     if (!Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
                        TT_ArrayInitializerLSquare,
-                       TT_DesignatedInitializerLSquare))
+                       TT_DesignatedInitializerLSquare, TT_AttributeSquare))
       return 500;
   }
 
@@ -2375,11 +2417,12 @@ bool TokenAnnotator::spaceRequiredBetwee
                                                      Style)) ||
             (Style.SpacesInSquareBrackets &&
              Right.MatchingParen->isOneOf(TT_ArraySubscriptLSquare,
-                                          TT_StructuredBindingLSquare)));
+                                          TT_StructuredBindingLSquare)) ||
+            Right.MatchingParen->is(TT_AttributeParen));
   if (Right.is(tok::l_square) &&
       !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
                      TT_DesignatedInitializerLSquare,
-                     TT_StructuredBindingLSquare) &&
+                     TT_StructuredBindingLSquare, TT_AttributeSquare) &&
       !Left.isOneOf(tok::numeric_constant, TT_DictLiteral))
     return false;
   if (Left.is(tok::l_brace) && Right.is(tok::r_brace))
@@ -2391,7 +2434,8 @@ bool TokenAnnotator::spaceRequiredBetwee
   if (Left.is(TT_BlockComment))
     return !Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
-    if (Left.is(tok::r_paren) && Left.is(TT_AttributeParen))
+    if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
+        (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
       return true;
     return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
            (Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
@@ -2602,6 +2646,8 @@ bool TokenAnnotator::spaceRequiredBefore
       return false;
     if (Right.is(TT_DictLiteral))
       return Style.SpacesInContainerLiterals;
+    if (Right.is(TT_AttributeColon))
+      return false;
     return true;
   }
   if (Left.is(TT_UnaryOperator))
@@ -2963,7 +3009,8 @@ bool TokenAnnotator::canBreakBefore(cons
     return !Right.isOneOf(tok::l_brace, tok::semi, tok::equal, tok::l_paren,
                           tok::less, tok::coloncolon);
 
-  if (Right.is(tok::kw___attribute))
+  if (Right.is(tok::kw___attribute) ||
+      (Right.is(tok::l_square) && Right.is(TT_AttributeSquare)))
     return true;
 
   if (Left.is(tok::identifier) && Right.is(tok::string_literal))
@@ -3004,6 +3051,9 @@ bool TokenAnnotator::canBreakBefore(cons
       (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None ||
        Left.getPrecedence() == prec::Assignment))
     return true;
+  if ((Left.is(TT_AttributeSquare) && Right.is(tok::l_square)) ||
+      (Left.is(tok::r_square) && Right.is(TT_AttributeSquare)))
+    return false;
   return Left.isOneOf(tok::comma, tok::coloncolon, tok::semi, tok::l_brace,
                       tok::kw_class, tok::kw_struct, tok::comment) ||
          Right.isMemberAccess() ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=327284&r1=327283&r2=327284&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Mar 12 08:42:38 2018
@@ -6067,6 +6067,16 @@ TEST_F(FormatTest, UnderstandsAttributes
                AfterType);
 }
 
+TEST_F(FormatTest, UnderstandsSquareAttributes) {
+  verifyFormat("SomeType s [[unused]] (InitValue);");
+  verifyFormat("SomeType s [[gnu::unused]] (InitValue);");
+  verifyFormat("SomeType s [[using gnu: unused]] (InitValue);");
+  verifyFormat("[[gsl::suppress(\"clang-tidy-check-name\")]] void f() {}");
+  verifyFormat("void f() [[deprecated(\"so sorry\")]];");
+  verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
+               "    [[unused]] aaaaaaaaaaaaaaaaaaaaaaa(int i);");
+}
+
 TEST_F(FormatTest, UnderstandsEllipsis) {
   verifyFormat("int printf(const char *fmt, ...);");
   verifyFormat("template <class... Ts> void Foo(Ts... ts) { Foo(ts...); }");
@@ -12088,29 +12098,34 @@ TEST_F(FormatTest, FileAndCode) {
   EXPECT_EQ(FormatStyle::LK_ObjC, guessLanguage("foo", "@interface Foo\n at end\n"));
 }
 
-TEST_F(FormatTest, GuessLanguageWithForIn) {
-  EXPECT_EQ(FormatStyle::LK_Cpp,
-            guessLanguage("foo.h", "for (Foo *x = 0; x != in; x++) {}"));
+TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
+  EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[noreturn]];"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
-            guessLanguage("foo.h", "for (Foo *x in bar) {}"));
-  EXPECT_EQ(FormatStyle::LK_ObjC,
-            guessLanguage("foo.h", "for (Foo *x in [bar baz]) {}"));
-  EXPECT_EQ(FormatStyle::LK_ObjC,
-            guessLanguage("foo.h", "for (Foo *x in [bar baz:blech]) {}"));
+            guessLanguage("foo.h", "array[[calculator getIndex]];"));
+  EXPECT_EQ(FormatStyle::LK_Cpp,
+            guessLanguage("foo.h", "[[noreturn, deprecated(\"so sorry\")]];"));
   EXPECT_EQ(
-      FormatStyle::LK_ObjC,
-      guessLanguage("foo.h", "for (Foo *x in [bar baz:blech, 1, 2, 3, 0]) {}"));
+      FormatStyle::LK_Cpp,
+      guessLanguage("foo.h", "[[noreturn, deprecated(\"gone, sorry\")]];"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
-            guessLanguage("foo.h", "for (Foo *x in [bar baz:^{[uh oh];}]) {}"));
+            guessLanguage("foo.h", "[[noreturn foo] bar];"));
   EXPECT_EQ(FormatStyle::LK_Cpp,
-            guessLanguage("foo.h", "Foo *x; for (x = 0; x != in; x++) {}"));
+            guessLanguage("foo.h", "[[clang::fallthrough]];"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
-            guessLanguage("foo.h", "Foo *x; for (x in y) {}"));
+            guessLanguage("foo.h", "[[clang:fallthrough] foo];"));
+  EXPECT_EQ(FormatStyle::LK_Cpp,
+            guessLanguage("foo.h", "[[gsl::suppress(\"type\")]];"));
+  EXPECT_EQ(FormatStyle::LK_Cpp,
+            guessLanguage("foo.h", "[[using clang: fallthrough]];"));
+  EXPECT_EQ(FormatStyle::LK_ObjC,
+            guessLanguage("foo.h", "[[abusing clang:fallthrough] bar];"));
+  EXPECT_EQ(FormatStyle::LK_Cpp,
+            guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
       FormatStyle::LK_Cpp,
-      guessLanguage(
-          "foo.h",
-          "for (const Foo<Bar>& baz = in.value(); !baz.at_end(); ++baz) {}"));
+      guessLanguage("foo.h",
+                    "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
+  EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
 }
 
 } // end namespace




More information about the cfe-commits mailing list