benhamilton added a comment.
Thanks very much for the code review. I fixed those issues.
================
Comment at: lib/Format/TokenAnnotator.cpp:323
+ const FormatToken *parseCpp11Attribute(const FormatToken *Tok,
+ bool NamespaceAllowed) {
----------------
jolesiak wrote:
> 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.
Fixed.
================
Comment at: lib/Format/TokenAnnotator.cpp:325
+ bool NamespaceAllowed) {
+ if (!Tok || !Tok->isOneOf(tok::identifier, tok::ellipsis)) return Tok;
+ Tok = Tok->Next;
----------------
jolesiak wrote:
> For consistency reasons I wouldn't use one line ifs. Let's keep existing
> format with line break and without braces.
Fixed. (My mistake was using `git-clang-format` without passing `--style llvm`
— for some reason that's applying a different style.)
================
Comment at: lib/Format/TokenAnnotator.cpp:337
+ ParamToken = ParamToken->Next;
+ if (!ParamToken || ParamToken->isNot(tok::r_paren)) return nullptr;
+ Tok = ParamToken->Next;
----------------
jolesiak wrote:
> Is second part of this condition necessary?
Not needed. Removed.
================
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;
----------------
jolesiak wrote:
> Pointer should be to const type, but same as above, I feel like const& would
> be a better choice.
Fixed.
================
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,
----------------
jolesiak wrote:
> 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;
> }
> ```
Fixed.
Repository:
rC Clang
https://reviews.llvm.org/D43902
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
