djasper added inline comments.
================ Comment at: lib/Format/TokenAnnotator.cpp:329 + return nullptr; + // C++17 '[[using namespace: foo, bar(baz, blech)]]' + bool IsUsingNamespace = ---------------- Can you make this: // C++17 '[[using <namespace>: foo, bar(baz, blech)]]' To make clear that we are not looking for kw_namespace here? ================ Comment at: lib/Format/TokenAnnotator.cpp:332 + AttrTok->startsSequence(tok::kw_using, tok::identifier, tok::colon); + if (IsUsingNamespace) { + AttrTok = AttrTok->Next->Next->Next; ---------------- No braces. ================ Comment at: lib/Format/TokenAnnotator.cpp:336 + auto parseCpp11Attribute = [](const FormatToken &Tok, + bool AllowNamespace) -> const FormatToken * { + if (!Tok.isOneOf(tok::identifier, tok::ellipsis)) ---------------- Do you actually need to put the return type here? I would have thought that it can be deduced as you pass back a const FormatToken* from a codepath and nullptr from all the others. ================ Comment at: lib/Format/TokenAnnotator.cpp:342 + return nullptr; + if (AllowNamespace && + AttrTok->startsSequence(tok::coloncolon, tok::identifier)) { ---------------- No braces. ================ Comment at: lib/Format/TokenAnnotator.cpp:350 + const FormatToken *ParamToken = AttrTok->Next; + while (ParamToken && ParamToken->isNot(tok::r_paren)) + ParamToken = ParamToken->Next; ---------------- Sorry that I have missed this before, I thought this was in a different file. Can you try to avoid iterating trying to count or match parentheses inside any of parseSquare/parseParen/parseAngle. We avoided that AFAICT for everything else and I don't think it's necessary here. Especially as you are not actually moving the token position forward, it's too easy to create a quadratic algorithm here. Also: Do you actually have a test case for the the parentheses case? This thing could use a lot more comments... ================ Comment at: lib/Format/TokenAnnotator.cpp:366 + return AttrTok->Next; + } else { + return nullptr; ---------------- No braces for single statement ifs. Don't use "else" after "return". ================ Comment at: lib/Format/TokenAnnotator.cpp:396 + while (CurrentToken != Cpp11AttributeSpecifierClosingRSquare) { + if (CurrentToken->is(tok::colon)) { + CurrentToken->Type = TT_AttributeColon; ---------------- No braces for single-statement ifs. ================ Comment at: lib/Format/TokenAnnotator.cpp:397 + if (CurrentToken->is(tok::colon)) { + CurrentToken->Type = TT_AttributeColon; + } ---------------- What happens if you don't assign this type here? Which formatting decision is based on it? ================ Comment at: unittests/Format/FormatTest.cpp:6064 + verifyFormat("SomeType s [[unused]] (InitValue);"); + verifyFormat("SomeType s [[gnu::unused]] (InitValue);"); + verifyFormat("SomeType s [[using gnu: unused]] (InitValue);"); ---------------- If this is meant to contrast a TT_AttributeColon from a different colon, that doesn't work. "::" is it's own token type coloncolon. 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