owenpan added inline comments.
================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:766 + assert(!Tokens.empty()); + const auto *LastToken = Tokens.back().Tok; + assert(LastToken); ---------------- curdeius wrote: > It might be a matter of taste but adding this variable makes the code harder > to read to me. The last token of `ParsedLine` is of interest here. We want the annotator to compute its `TotalLength` to determine whether the line might fit on a single line. I'm open to renaming the variable if you have a better suggestion. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:777 + SavedToken.Tok->copyFrom(*Token.Tok); + SavedToken.Children = std::move(Token.Children); + } ---------------- curdeius wrote: > So the token's children are modified (moved)? Is it done so that children be > not considered by the annotator? Both the constructor and the destructor of `AnnotatedLine` clear the children of `UnwrappedLineNode`, so we save them beforehand and restore them afterward. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787 + + const int Length = LastToken->TotalLength; + ---------------- curdeius wrote: > Why not like this? > Why not like this? See the assertion on line 781 above. We are computing the `TotalLength` of `LastToken` via `Line`. Either way works, but I prefer the simpler expression. I can change it though if you insist. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125137/new/ https://reviews.llvm.org/D125137 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
