curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed.
Last nits, apart from this clean up, I'm OK. ================ Comment at: clang/lib/Format/FormatToken.h:125 +/// Operators that can follow a C variable. +static const std::set<clang::tok::TokenKind> C_OperatorsFollowingVar = { + tok::l_square, tok::r_square, ---------------- HazardyKnusperkeks wrote: > And maybe choose a different container: > https://llvm.org/docs/ProgrammersManual.html#set-like-containers-std-set-smallset-setvector-etc Not done it seems. Please rename and use a different type. Maybe `ImmutableSet`? Or just a sorted `std::vector`? ================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:109 + return true; + // Handle Qt signals + else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) && ---------------- Nit here and elsewhere: full stop at the end of comments. ================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:113 + return true; + // Handle malformed access specifier i.e. 'private' without trailing ':' + else if ((RootToken.isAccessSpecifier(false) && ---------------- ================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:116 + (!RootToken.Next || + (!C_OperatorsFollowingVar.count( + RootToken.Next->Tok.getKind()) && ---------------- Please use `contains` instead of `count` if using `ImmutableSet`. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2497 void UnwrappedLineParser::parseAccessSpecifier() { + auto *AccessSpecifierCandidate = FormatTok; nextToken(); ---------------- ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2506-2512 + // is not a variable name or namespacename + } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) && + !FormatTok->Tok.is(tok::coloncolon)) + addUnwrappedLine(); + // consider the accessSpecifier to be a C identifier + else if (AccessSpecifierCandidate) + AccessSpecifierCandidate->Tok.setKind(tok::identifier); ---------------- ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2507 + // is not a variable name or namespacename + } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) && + !FormatTok->Tok.is(tok::coloncolon)) ---------------- Ditto. Use `contains`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117416/new/ https://reviews.llvm.org/D117416 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits