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

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 1 09:45:01 PST 2023


aaron.ballman added a comment.

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

> In D139774#4096695 <https://reviews.llvm.org/D139774#4096695>, @aaron.ballman wrote:
>
>> There's three scenarios when a field is added to the structure: 1) library and app are matching versions, 2) library is newer than app, 3) app is newer than library. #1 is the happy path most folks will hopefully be on. For #2 and #3, the app will either be sending more or less data than the library expects, but the library will know how much of the structure is valid based on the size field. If the size is too small and the library can't get data it needs, it can catch that and report an error instead of reading past a buffer. And if the size is too large, the library can ignore anything it doesn't know about or it can give an error due to not knowing how to support the semantics of the newer interface.
>
> In the second case, the library ideally should assume that the missing struct members are not specified and behave as the corresponding older version.

I think agree.

> In the third case, the app can support older libclang versions by passing successively smaller structs until libclang returns a valid `CIndex`.

That's how it tends to work when I've used APIs like this before (this is a pretty common pattern in the Win32 C APIs).

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

> Perhaps the struct should contain the value of `CINDEX_VERSION_MINOR` instead of the `sizeof`? This way, libclang can change the meaning of a struct member without changing the size of the struct. For example, replace `PreambleStoragePath` with `TemporaryDirectoryPath`. Such a change could even be quiet, because setting the more general temporary directory path would likely be welcome or at least acceptable to the API users.

We try to keep libclang APIs source and ABI stable; changing the name or meaning of the structure member would defeat source stability. We could still use the minor version regardless, but then the minor version must continue to be monotonic even if we bump the major version (which we said we expect not to do, but "expect not to" != "will never").


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