[PATCH] D63672: Added Delta IR Reduction Tool

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/tools/llvm-reduce/TestRunner.cpp:46-47
+
+  StringRef SR = "";
+  Optional<StringRef> Redirects[3] = {SR, SR, SR}; // STDIN, STDOUT, STDERR
+  int Result = sys::ExecuteAndWait(TestName, ProgramArgs, None, Redirects);
----------------
Do you need to initialize these elements? I suspect just this:

  Optional<StringRef> Redirects[3];

should suffice (since they're "Optional" - it'd be weird to have to make them "non-None but empty strings").


================
Comment at: llvm/tools/llvm-reduce/deltas/Delta.cpp:168-169
+    // Delete uninteresting chunks
+    auto UnwantedChunks = Chunks.end();
+    UnwantedChunks = std::remove_if(Chunks.begin(), Chunks.end(),
+                                    [UninterestingChunks](const Chunk &C) {
----------------
Looks like an unnecessary initialization? Coeuld be written as:

  auto UnwantedChunks = std::remove_if....

Though some people would actually wrap up erase and remove together:

  Chunks.erase(llvm::remove_if(Chunks, [&](const Chunk &C) {
    return UninterestingChunks.count(C);
  }, Chunks.end());

Oh, there we go, we have an llvm::erase_if:

  erase_if(Chunks, [&](const Chunk &C) {
    ...
  });

Nice & easy :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63672