<p dir="ltr">What do you think about the below interface changes? It seems like they would address most of the concerns you're raising here.</p>
<p dir="ltr">typedef std::function<AddStreamFn(unsigned Task, StringRef Key)>  NativeObjectCache;</p>
<p dir="ltr">NativeObjectCache localCache(std::string CacheDirectoryPath, AddFileFn AddFile);</p>
<p dir="ltr">Error LTO::run(AddStreamFn AddStream, NativeObjectCache Cache) </p>
<p dir="ltr">And remove Cache field from config.</p>
<p dir="ltr">Peter</p>
<div class="gmail_extra"><br><div class="gmail_quote">On Sep 22, 2016 10:25 PM, "Mehdi AMINI" <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mehdi_amini added inline comments.<br>
<br>
================<br>
Comment at: include/llvm/LTO/Config.h:60<br>
@@ +59,3 @@<br>
+/// cache, pass a unique string as the Key. For hits, the cached file will be<br>
+/// added using AddFile and this function will return AddStreamFn(). For misses,<br>
+/// the cache will return a stream callback which must be called at most once to<br>
----------------<br>
The "AddFile" part seems convoluted to me. It seems to be here only to satisfy the flow of having a stream as a returned type.<br>
<br>
================<br>
Comment at: include/llvm/LTO/Config.h:72<br>
@@ -64,2 +71,3 @@<br>
+    NativeObjectCache;<br>
<br>
 /// LTO configuration. A linker can configure LTO by setting fields in this data<br>
----------------<br>
Since you're claiming to make it easier to change the cache implementation in the description, can you clarify how would I implement a "in-memory" cache with this interface?<br>
<br>
================<br>
Comment at: include/llvm/LTO/Config.h:116<br>
@@ -107,1 +115,3 @@<br>
<br>
+  NativeObjectCache Cache;<br>
+<br>
----------------<br>
No documented.<br>
<br>
================<br>
Comment at: include/llvm/LTO/LTO.h:316<br>
@@ +315,3 @@<br>
+  /// AddFile functions to add native object files to the link.<br>
+  ///<br>
+  /// The client will receive at most one callback (via either AddStream or<br>
----------------<br>
> Is there any specific client you have in mind where this would be a problem?<br>
<br>
That's not the angle I'm looking at when looking at the API design.<br>
<br>
For instance:<br>
Why is `NativeObjectCache Cache` in in the config? While these callbacks are here?<br>
<br>
<br>
<br>
<a href="https://reviews.llvm.org/D24622" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24622</a><br>
<br>
<br>
<br>
</blockquote></div></div>