HazardyKnusperkeks added a comment.
Looks good to me, apart from that one issue.
================
Comment at: clang/lib/Tooling/Inclusions/IncludeStyle.cpp:23
+
+constexpr int DefaultPriority = std::numeric_limits<int>::max();
+
----------------
Please move this into the `mapping` function.
================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:11
#include "clang/Format/Format.h"
+#include "clang/Tooling/Inclusions/IncludeStyle.h"
#include "llvm/ADT/StringRef.h"
----------------
Don't need that, or?
================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:548
+ EXPECT_EQ("#include \"a.h\"\n"
+ "#include \"llvm/a.h\"\n"
+ "#include \"b.h\"\n"
----------------
While I may accept there are more than one //main// header, this should not
happen in my opinion.
================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:557
+
+ // Keep manually splitted groups in place
EXPECT_EQ("#include \"a.h\"\n"
----------------
End comments with full stop.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143691/new/
https://reviews.llvm.org/D143691
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
