[PATCH] D139774: [libclang] Add API to set temporary directory location

Fri Jan 20 10:38:43 PST 2023

dblaikie added a comment.

In D139774#4069014 <https://reviews.llvm.org/D139774#4069014>, @aaron.ballman wrote:

> In D139774#4066920 <https://reviews.llvm.org/D139774#4066920>, @dblaikie wrote:
>
>> Don't let me hold this up - I think it all feels a bit too ad-hoc for my own preferences (feels like there should be fairly general solutions to this - rather than playing whack-a-mole on only the biggest temporary files - both in terms of the options KDevelop developers have considered/accepted/declined, and in terms of what's being proposed to be added to Clang), but I don't feel strongly enough about any of that to veto/push very hard here.
>
> I think the general solution would be what's proposed here -- allow overriding the system temp directory at the LLVM filesystem API level. But that continues to feel wrong to me -- asking for the system temp directory should get you the system temp directory. For example, nothing says that libclang's need for the temp directory is the same as someone else using the LLVM filesystem APIs within the same library but for totally unrelated purposes. It seems like the wrong layer at which to add this logic -- the base API should get you the system directory, and the caller should be responsible for deciding if they wanted something different than that or not. I realize that turns into a bit of a game of whack-a-mole to find all of the places where we make such decisions, but that seems like the more correct approach to me. If we want this to be controlled by the programmer (and it sounds like KDevelop has very good reasons to want this to be controlled), I think it should become a part of the API interface rather than a hidden one-time setter that impacts the entire process.

I think the only problem I have with changing the lower layer is the race condition/making decisions for totally unrelated clients. If there was some "LLVMSystemContext" that was passed into every LLVM API & one client could create/configure that with one temp directory and use it for LLVM, Clang, etc, and another client in the same process could use another, that'd be fine. I guess that would allow the independent configuration, but also the singular configuration when that's what the client desires. But that's impractical/would be too much engineering for this issue.

*shrug* Ah well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774