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

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak requested changes to this revision.
jolesiak added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Format/TokenAnnotator.cpp:323
 
+  const FormatToken *parseCpp11Attribute(const FormatToken *Tok,
+                                         bool NamespaceAllowed) {
----------------
I feel like it would be clearer (and more consistent) if we used const& here and just make a copy inside function body.
I see no benefit of passing nullptr based on usage presented in this patch.


================
Comment at: lib/Format/TokenAnnotator.cpp:325
+                                         bool NamespaceAllowed) {
+    if (!Tok || !Tok->isOneOf(tok::identifier, tok::ellipsis)) return Tok;
+    Tok = Tok->Next;
----------------
For consistency reasons I wouldn't use one line ifs. Let's keep existing format with line break and without braces.


================
Comment at: lib/Format/TokenAnnotator.cpp:337
+        ParamToken = ParamToken->Next;
+      if (!ParamToken || ParamToken->isNot(tok::r_paren)) return nullptr;
+      Tok = ParamToken->Next;
----------------
Is second part of this condition necessary?


================
Comment at: lib/Format/TokenAnnotator.cpp:345
+  // never a valid Objective-C or Objective-C++ method invocation.
+  bool parseCpp11AttributeSpecifier(FormatToken *Tok) {
+    if (!Style.isCpp()) return false;
----------------
Pointer should be to const type, but same as above, I feel like const& would be a better choice.


================
Comment at: lib/Format/TokenAnnotator.cpp:351
+    if (!AttributeToken) return false;
+    // C++17 '[[using namespace: foo, bar(baz, blech)]]'
+    if (AttributeToken->startsSequence(tok::kw_using, tok::identifier,
----------------
How about:

```
    bool NamespaceAllowed;
    if (AttributeToken->startsSequence(tok::kw_using, tok::identifier,
                                       tok::colon)) {
      // C++17 '[[using namespace: foo, bar(baz, blech)]]'
      AttributeToken = AttributeToken->Next->Next->Next;
      NamespaceAllowed = false;
    } else {
      // C++11 '[[namespace::foo, namespace::bar(baz, blech)]]'
      NamespaceAllowed = true;
    }
    while (AttributeToken) {
      AttributeToken =
          parseCpp11Attribute(AttributeToken, NamespaceAllowed);
      if (!AttributeToken || AttributeToken->isNot(tok::comma)) break;
      AttributeToken = AttributeToken->Next;
    }
```


Repository:
  rC Clang

https://reviews.llvm.org/D43902