[PATCH] D65026: [Bugpoint redesign] Added pass to reduce Metadata

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/tools/llvm-reduce/DeltaManager.h:19-40
+
+namespace {
+static ManagedStatic<std::vector<std::string>> DeltaPasses;
+
+struct DeltaPassOption {
+  void operator=(const std::string &Val) const {
+    if (Val.empty())
----------------
Anonymous namespaces and static variables aren't suitable for header files (they can lead to ODR violations, etc) You can inline these things inside runDeltaPasses (using lambdas for local functions, etc) or take the runDeltaPasses definition out of line into a .cc file and move these static variables/anonymous namespace content to there.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceGlobalVars.cpp:15
 
-#include "RemoveGlobalVars.h"
+#include "ReduceGlobalVars.h"
 
----------------
Might've been a bit easier to do some of these renames as separate patches - super easy to review (or commit without review) & keeps the "interesting" reviews uncluttered.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceMetadata.cpp:20
+static void getChunkMetadataNodes(T *MDUser, int &I,
+                                  std::vector<Chunk> ChunksToKeep,
+                                  std::set<MDNode *> &SeenNodes,
----------------
Is this intentionally passed by value for some reason? (usually a non-trivial data structure like a std::vector would be passed by const ref if a copy isn't needed, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65026