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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 1 07:43:14 PST 2023


aaron.ballman added a comment.

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

> In D139774#4096527 <https://reviews.llvm.org/D139774#4096527>, @aaron.ballman wrote:
>
>> That sounds like a good plan to me. I wonder if we want to name it something like `clang_createIndexWithOptions` (or something generic like that), and give it a versioned structure of options along the lines of:
>>
>>   struct CIndexOptions {
>>     uint32_t Size; // sizeof(struct CIndexOptions), used for option versioning
>>     const char *PreambleStoragePath; // This pointer can be freed after creating the index
>>   };
>>
>> and define the function to return an error if `Size < sizeof(struct CIndexOptions)`. This should allow us to add additional options to the structure without having to introduce a new constructor API each time.
>
> Would this be the recommended usage of such an API?
>
> 1. Call `clang_createIndexWithOptions`.
> 2. If it returns `nullptr` index, report an error in the application (e.g. print a warning or show an error in the UI) and fall back to code paths that support older Clang versions, beginning with calling the older constructor `clang_createIndex`.

Yeah, I would imagine robust code to look like:

  CIndexOptions Opts;
  Opts.Size = sizeof(Opts);
  Opts.PreambleStoragePath = somePathPtr;
  CIndex Idx = clang_createIndexWithOptions(/*excludeDeclsFromPCH=*/1, /*displayDiagnostics=*/1, &Opts);
  if (!Idx) {
    Idx = clang_createIndex(/*excludeDeclsFromPCH=*/1, /*displayDiagnostics=*/1);
    if (!Idx)
      handleError();
  }

> Is assigning `sizeof(CIndexOptions)` to `Size` the API user's responsibility or should libclang define an inline function `clang_initializeCIndexOptions`?

The user's responsibility. 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.


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