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

Igor Kushnir via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 4 09:55:33 PST 2023


vedgy added a comment.

In D139774#4025356 <https://reviews.llvm.org/D139774#4025356>, @vedgy wrote:

> `clang_disposeIndex()` would have to un-override the temporary directory then. I'll have to check whether this un-overriding happens too early (before all //preamble-*.pch// files are removed).

Checked by calling `clang_setTemporaryDirectory(nullptr);` right after `clang_disposeIndex(m_index);`. Works correctly in KDevelop: all //preamble-*.pch// files are removed from the overriding temporary directory on exit. So this alternative API is viable.



================
Comment at: clang/include/clang-c/Index.h:78-79
+ * this function in order to reset the temporary directory to the default value
+ * from the environment. Such a resetting should be done before deleting a
+ * tempDirUtf8 pointer previously passed to this function.
+ */
----------------
vedgy wrote:
> aaron.ballman wrote:
> > Should we mention anything about relative vs absolute path requirements? Separators? 
> On Windows separators are converted automatically. I suppose we don't need to warn users not to pass Windows-style separators on Unix.
> 
> On Windows this function handles both relative and absolute paths. On Unix the existing implementation doesn't check whether the path is absolute or relative. Perhaps it assumes that the path in the environment variable is always absolute. Or absolute vs relative doesn't matter. I'll check what happens if a relative path is passed.
Tested passing a relative path on GNU/Linux: works as expected. libclang stores and removes the //preamble-*.pch// in the same absolute path as [QDir::absolutePath()](https://doc.qt.io/qt-5/qdir.html#absolutePath) returns in KDevelop. Since passing any reasonably formatted path to this function works, I suppose mentioning the path requirements in the documentation is unnecessary.

While testing, I noticed another requirement that **should** be mentioned in this documentation, as well as `set_system_temp_directory_erased_on_reboot`'s documentation: the temporary directory must exist, because Clang doesn't create it. I couldn't find where Clang writes the preamble files when the temporary directory does not exist. Perhaps it doesn't write them to disk at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774



More information about the cfe-commits mailing list