[PATCH] D24622: LTO: Simplify caching interface.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: include/llvm/LTO/Config.h:60
@@ +59,3 @@
+/// cache, pass a unique string as the Key. For hits, the cached file will be
+/// added using AddFile and this function will return AddStreamFn(). For misses,
+/// the cache will return a stream callback which must be called at most once to
----------------
mehdi_amini wrote:
> 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.
Now gone

================
Comment at: include/llvm/LTO/Config.h:72
@@ -64,2 +71,3 @@
+    NativeObjectCache;
 
 /// LTO configuration. A linker can configure LTO by setting fields in this data
----------------
mehdi_amini wrote:
> 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?
New interface would allow you to implement that within the cache either internally or with a new callback.

================
Comment at: include/llvm/LTO/Config.h:116
@@ -107,1 +115,3 @@
 
+  NativeObjectCache Cache;
+
----------------
mehdi_amini wrote:
> No documented.
Removed.

================
Comment at: include/llvm/LTO/LTO.h:316
@@ +315,3 @@
+  /// AddFile functions to add native object files to the link.
+  ///
+  /// The client will receive at most one callback (via either AddStream or
----------------
mehdi_amini wrote:
> > Is there any specific client you have in mind where this would be a problem?
> 
> That's not the angle I'm looking at when looking at the API design.
> 
> For instance:
> Why is `NativeObjectCache Cache` in in the config? While these callbacks are here? 
> 
This is resolved with the new interface.


https://reviews.llvm.org/D24622