djasper added a comment.
In general, this is lacking test cases for imports that are wrapped over
multiple lines to start with. Should probably add those both for the old and
for the new behavior.
================
Comment at: lib/Format/Format.cpp:1229
@@ -1227,9 +1228,3 @@
// the entire block. Otherwise, no replacement is generated.
- bool OutOfOrder = false;
- for (unsigned i = 1, e = Indices.size(); i != e; ++i) {
- if (Indices[i] != i) {
- OutOfOrder = true;
- break;
- }
- }
- if (!OutOfOrder)
+ bool InOrder = std::is_sorted(Indices.begin(), Indices.end());
+ if (InOrder)
----------------
I think it's not even worth storing this variable now.
================
Comment at: lib/Format/SortJavaScriptImports.cpp:46
@@ +45,3 @@
+
+ bool operator==(const JsImportedSymbol &RHS) const {
+ return Symbol == RHS.Symbol && Alias == RHS.Alias;
----------------
Maybe add a comment on why you are ignoring 'Range' for comparison.
================
Comment at: lib/Format/SortJavaScriptImports.cpp:168
@@ +167,3 @@
+
+ if (ReferencesInOrder && SymbolsInOrder) {
+ return Result;
----------------
nit: no braces
================
Comment at: lib/Format/SortJavaScriptImports.cpp:232
@@ +231,3 @@
+ std::stable_sort(Symbols.begin(), Symbols.end(),
+ [&](JsImportedSymbol LHS, JsImportedSymbol RHS) {
+ return LHS.Symbol < RHS.Symbol;
----------------
I think it's safe to pass the symbols as const reference here.
http://reviews.llvm.org/D20798
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
