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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
