[PATCH] D31020: Support: Add a cache pruning policy parser.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

Neat! LGTM.



================
Comment at: llvm/include/llvm/LTO/Caching.h:23
+
+struct CachePruningPolicy;
+
----------------
Do all the clients of this header need this declaration? I found strange that nothing changes in the header but this is needed.


================
Comment at: llvm/include/llvm/Support/CachePruning.h:47
+/// Parse the given string as a cache pruning policy. Defaults are taken from a
+/// default constructed CachePruningPolicy object.
+Expected<CachePruningPolicy> parseCachePruningPolicy(StringRef PolicyStr);
----------------
Could you add an example syntax here?


================
Comment at: llvm/lib/Support/CachePruning.cpp:37
 
+static Expected<std::chrono::seconds> parseInterval(StringRef Interval) {
+  if (Interval.empty())
----------------
Why "Interval"? It seems that you're parsing a "time" or a "duration"?


================
Comment at: llvm/lib/Support/CachePruning.cpp:98
+                                     inconvertibleErrorCode());
+    }
+  }
----------------
StringSwitch? I guess it does not play well with the error to return...


https://reviews.llvm.org/D31020