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

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

Reply via email to