[PATCH] D145098: [clang][deps] Preserve input ordering in the full output

Ben Langmuir via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 1 11:54:06 PST 2023


benlangmuir added inline comments.


================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:291
 
-    std::unique_lock<std::mutex> ul(Lock);
-    Inputs.push_back(std::move(ID));
+    Inputs[InputIndex] = std::move(ID);
   }
----------------
Since the input index is coming from "outside": does this `operator[]` assert if the index is out of range? If not, we should do so here.  I would also suggest asserting the value isn't already populated (e.g. `assert(ID.FileName.empty())`).


================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:788
+  if (Format == ScanningOutputFormat::Full && ModuleName.empty())
+    FD.resize(Inputs.size());
+
----------------
Since you should never resize `FullDeps` after you start, how about removing `resize` and instead:
* Make `FullDeps` constructor take the size
* Change `FullDeps FD;` to `std::optional<FullDeps>`
* Change this line to `FullDeps.emplace(Inputs.size())`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145098/new/

https://reviews.llvm.org/D145098



More information about the cfe-commits mailing list