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

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 07:39:00 PST 2023


aaron.ballman added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774



More information about the llvm-commits mailing list