[PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 2 08:44:19 PST 2017


alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

If you're still considering to submit this patch, could you rebase it (or maybe re-generate instead?) and split into easier to digest parts?

A couple of things I noticed:

1. `v.push_back(X);` -> `v.emplace_back(X);` pattern, where `X` has a type of element of `v`. Not sure whether `emplace_back` provides any benefit in this case.
2. a sub-case of 1 where `X` is `std::make_pair(...)`, in this case `emplace_back` makes sense, if `std::make_pair` is removed as well. I don't know whether it's practical to teach the check this pattern. Given its frequency, might well be good.



================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:3903
       if (Record.size() > 10 && Record[10] != 0)
-        FunctionPrologues.push_back(std::make_pair(Func, Record[10]-1));
+        FunctionPrologues.emplace_back(std::make_pair(Func, Record[10]-1));
 
----------------
No `make_pair`.


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:3921
       if (Record.size() > 13 && Record[13] != 0)
-        FunctionPrefixes.push_back(std::make_pair(Func, Record[13]-1));
+        FunctionPrefixes.emplace_back(std::make_pair(Func, Record[13]-1));
 
----------------
ditto


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:3924
       if (Record.size() > 14 && Record[14] != 0)
-        FunctionPersonalityFns.push_back(std::make_pair(Func, Record[14] - 1));
+        FunctionPersonalityFns.emplace_back(std::make_pair(Func, Record[14] - 1));
 
----------------
ditto


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:3989
       ValueList.push_back(NewGA);
-      IndirectSymbolInits.push_back(std::make_pair(NewGA, Val));
+      IndirectSymbolInits.emplace_back(std::make_pair(NewGA, Val));
       break;
----------------
ditto


================
Comment at: lib/Bitcode/Writer/ValueEnumerator.cpp:149
     if (OM.lookup(U.getUser()).first)
-      List.push_back(std::make_pair(&U, List.size()));
+      List.emplace_back(std::make_pair(&U, List.size()));
 
----------------
No `std::make_pair` needed.


================
Comment at: lib/Bitcode/Writer/ValueEnumerator.cpp:606
       else
-        Worklist.push_back(std::make_pair(Op, Op->op_begin()));
+        Worklist.emplace_back(std::make_pair(Op, Op->op_begin()));
       continue;
----------------
ditto


================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:50
   }
-  Ranges.push_back(std::make_pair(&MI, nullptr));
+  Ranges.emplace_back(std::make_pair(&MI, nullptr));
 }
----------------
No need for `std::make_pair`.


================
Comment at: lib/CodeGen/CGExprScalar.cpp:2755
       BaseCheck->addIncoming(ValidBase, CheckShiftBase);
-      Checks.push_back(std::make_pair(BaseCheck, SanitizerKind::ShiftBase));
+      Checks.emplace_back(std::make_pair(BaseCheck, SanitizerKind::ShiftBase));
     }
----------------
`make_pair` can be removed, IIUC.


================
Comment at: lib/CodeGen/CGVTables.cpp:948
   for (auto &&AP : VTLayout.getAddressPoints())
-    BitsetEntries.push_back(std::make_pair(AP.first.getBase(), AP.second));
+    BitsetEntries.emplace_back(std::make_pair(AP.first.getBase(), AP.second));
 
----------------
Not sure if it's practical to teach the check this pattern, but `std::make_pair` is not needed here.


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:1498
   void visitCrossEdge(const SDep &PredDep, const SUnit *Succ) {
-    ConnectionPairs.push_back(std::make_pair(PredDep.getSUnit(), Succ));
+    ConnectionPairs.emplace_back(std::make_pair(PredDep.getSUnit(), Succ));
   }
----------------
No `make_pair` needed.


================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:2079
       }
-      FuncInfo.PHINodesToUpdate.push_back(std::make_pair(MBBI++, Reg));
+      FuncInfo.PHINodesToUpdate.emplace_back(std::make_pair(MBBI++, Reg));
       DbgLoc = DebugLoc();
----------------
No `make_pair` needed.


Repository:
  rL LLVM

https://reviews.llvm.org/D21507





More information about the cfe-commits mailing list