curdeius requested changes to this revision.
curdeius added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/lib/Format/FormatToken.h:125
+/// Sorted Operators that can follow a C variable.
+static const std::vector<clang::tok::TokenKind> COperatorsFollowingVar = {
----------------
================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:122-129
+ auto COperatorMatch = std::lower_bound(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(),
+ RootToken.Next->Tok.getKind());
+ if ((COperatorMatch == COperatorsFollowingVar.end() ||
+ *COperatorMatch != RootToken.Next->Tok.getKind()) &&
+ !RootToken.Next->Tok.is(tok::coloncolon)) {
+ return true;
----------------
HazardyKnusperkeks wrote:
> Use `std::binary_search` instead of `std::lower_bound`. That should simplify
> the following `if`.
Please do something along these lines. The idea is to put a cheaper check first.
Note. I haven't tested nor formatted these lines.
================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:905
/// break after the "{", format all lines with correct indentation and the
put
- /// the closing "}" on yet another new line.
+ /// the closing "}" on yet another new line.
///
----------------
Revert.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2716-2718
+ auto COperatorMatch =
+ std::lower_bound(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(), FormatTok->Tok.getKind());
----------------
Please use `binary_search` and put it inside the `else` branch to avoid it if
the first condition is satisfied.
Something like:
```
if (FormatTok->Tok.is(tok::colon)) {
...
} else if (!binary_search(...) {
} else if (...) {
}
```
Also, this code and the code in `UnwrappedLineFormatter` are pretty much
similar.
Can we remove this duplication by e.g. setting the token kind here and checking
it in the formatter?
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
