owenpan added inline comments.
================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3226 + // No space between module :. + if (Left.isOneOf(Keywords.kw_module, tok::kw_export, Keywords.kw_import) && + Right.is(TT_ModulePartitionColon)) ---------------- You can remove `kw_export` as it must be followed by `import` or `module`, based on how `TT_ModulePartitionColon` is set on Line 908. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3230-3231 + // No space between import foo:bar but keep a space between import :bar; + if (Left.isOneOf(tok::identifier, tok::kw_public, tok::kw_private) && + !Left.is(Keywords.kw_import) && Right.is(TT_ModulePartitionColon)) + return false; ---------------- You can drop `!Left.is(Keywords.kw_import)` as `import :bar` is already covered by Line 3226. Also, I would remove `kw_public` and `kw_private` as they are not special WRT other keywords when followed by `TT_ModulePartitionColon`. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3235 + if (Left.is(TT_ModulePartitionColon) && + Right.isOneOf(tok::identifier, tok::kw_public, tok::kw_private)) + return false; ---------------- Is `module :public` a thing in C++20? If not, I would remove `kw_public`. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3238 + // import .....; + if (Left.is(Keywords.kw_import) && Right.is(tok::ellipsis)) + return true; ---------------- You can fold this line into Line 3223 above. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1253 } + if (Style.isCpp()) { + nextToken(); ---------------- Maybe move this entire block into a function? ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1260 + // Handle import <foo/bar.h> as we would an include statement + if (FormatTok && FormatTok->is(tok::less)) { + nextToken(); ---------------- You can change this line to `else if (FormatTok->is(tok::less)) {` as `FormatTok` can't be null. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1262-1264 + while (FormatTok) { + if (FormatTok->is(tok::semi)) + break; ---------------- You can combine these three lines into one: `while (FormatTok && FormatTok->isNot(tok::semi)) {` ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1267-1268 + // literals. + if (FormatTok->isNot(tok::comment) && + !FormatTok->TokenText.startswith("//")) + FormatTok->setType(TT_ImplicitStringLiteral); ---------------- Don't you want to mark up to the matching `>`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://reviews.llvm.org/D114151 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits