vedgy marked an inline comment as not done.
vedgy added a comment.

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

> In terms of the C API, I think it'd make more sense to name in terms of 
> "override" rather than "set", but I don't feel as strongly about it given the 
> other setters. In terms of the C++ file system API, I think "override" makes 
> the most sense though (we don't have setters to follow the naming convention 
> for) because some systems do allow you to set the system directory.

Let's keep the naming in C and C++ APIs consistent: 
`clang_overrideTemporaryDirectory()` and 
`override_system_temp_directory_erased_on_reboot()`.

> In terms of memory ownership, WDYT of requiring the caller to handle this? 
> e.g., calling `set_system_temp_directory_erased_on_reboot` will `strdup` a 
> nonnull pointer and `free` the stored pointer when given nullptr.

I like this idea. libclang-user code would become easier to use than it is now 
(though less easy compared to libclang managing memory itself). The libclang 
API documentation can require overriding the temp directory before creating an 
index and un-overriding it with `nullptr` after calling `clang_disposeIndex()`.
Now in order to make this libclang API harder to misuse, I lean towards passing 
the temporary directory in `clang_createIndexWithTempDir()` and letting 
`clang_disposeIndex()` handle the un-overriding (call 
`override_system_temp_directory_erased_on_reboot(nullptr)`) automatically. 
Makes sense? I feel that the `clang_createIndexWithTempDir()` name could be 
improved, but don't know how...

What memory [de]allocation method should 
`override_system_temp_directory_erased_on_reboot()` use? `new[]` and 
`delete[]`? Or should `strdup()` from POSIX be used because it is defined as 
`_strdup` on Windows? (along with standard `free()`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to