[PATCH] D41231: [Support][CachePruning] Fix regression that prevents disabling of pruning

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

In https://reviews.llvm.org/D41231#955453, @bd1976llvm wrote:

> I like your proposal.
>
> However, as the cache code is shared between multiple api's and only the c api has this disabling behaviour for negative values a fix here is more appropriate..


I don't quite agree with that conclusion -- the fact we have one upper layer that supports feature X means that the lower layer needs to support it as well (the other front-ends can just choose not to use it). If one front-end is relying on an undocumented quirk of a back-end to implement its feature (which the implicit signed-to-unsigned conversion was, and was probably the reason I did not notice this when doing the conversion) then you get problems.

That said, I don't think it's my place to ask you to do this extra work, and I agree that we should fix the regression. I just felt I should point out a potential issue with this approach, but I am leaving the final decision in the hands of the respective owner(s).

> I have updated the patch to be more defensive, hopefully this addresses your concerns.

Somewhat, but the conversion here was not my primary concern. I am more worried about some code down the line writing
`auto deadline = system_clock::now() /*nanoseconds*/ + CacheOptions.Policy.Interval /*implicitly converted to nanoseconds*/;`


https://reviews.llvm.org/D41231