[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: tools/libclang/CIndexCodeCompletion.cpp:309
+  /// before that result for the corresponding completion item.
+  std::vector<std::vector<FixItHint>> FixItsVector;
 };
----------------
yvvan wrote:
> ilya-biryukov wrote:
> > Storing `vector<vector<>>` here makes looks like a hack. Even though it might seem more tricky, I suggest storing an opaque pointer to `vector<FixItHint>` in each `CXCompletionResult`.  Managing the lifetime of vectors in the `AllocatedCXCodeCompleteResults` seems totally fine, but there should be a way to get to the fixits in a similar way we can get to the completion string.
> > More concretely, I suggest the following API:
> > ```
> > // === Index.h
> > typedef void* CXCompletionFixIts;
> > typedef struct {
> >    // ...
> >    CXCompletionFixIts FixIts;
> > };
> > 
> > // Get the number of fix-its.
> > unsigned clang_getCompletionNumFixIts(CXCompletionResult *Result);
> > // ... Similar to getDiagnosticFixIt
> > CXString clang_getCompletionFixIt((CXCompletionResult *Result, unsigned fixit_index, CXSourceRange *replacement_range);
> > 
> > 
> > 
> > // === Impl.cpp
> > struct AllocatedCXCodeCompleteResults : public CXCodeCompleteResults {
> > // .....
> > // Pool for allocating non-empty fixit vectors in the CXCompletionResult.
> > std::vector<std::vector<FixItHint>> FixItsVector
> > };
> > 
> > unsigned clang_getCompletionNumFixIts(CXCompletionResult *Result) {
> >   auto* FixIts = static_cast<std::vector<FixItHint>*>(Result->FixIts);
> >   if (!FixIts)
> >     return 0;
> >   return FixIts->size();
> > }
> > ```
> unsigned clang_getCompletionNumFixIts(CXCompletionResult *Result);
> 
> Do you mean appending CXCompletionResult struct with the "CXCompletionFixIts FixIts" member? Doesn't it break binary compatibility (changing the struct size)?
Ah, you're right. If libclang promises binary compatibility with binaries compiled from headers with previous versions, we're not allowed to do this change.
I'm not sure what's the best way to do that, probably the interface you propose does what we want with the least amount of friction.
I'd be fine with leaving as is, but given that `libclang` has a stable interface, I'd get more opinions from someone who owns the code before adding this. (Another reason to split this change into two?)

A possible alternative that won't break binary compatibility would be to change the opaque `CXCompletionString` to point into a struct that has both `CodeCompletionString` and the fixits vector, but that means finding all use-sites of `CXCompletionString` and that might be tricky.


https://reviews.llvm.org/D41537