DeinAlptraum wrote:

@Endilll I tried to implement point 5 from your issue, but I ran into trouble 
along the way:

When we fold `CCRStructure` into `CodeCompletionResults`, we need to add the 
relevant fields of the C-side `CXCodeCompleteResults` objects (which are 
currently kept by `CCRStructure`)  to `CodeCompletionResults` instead, but this 
is not possible without an immediate break in functionality because:
- `CodeCompletionResults` currently offers a `results` field (`property`) to 
access the underlying `CCRStructure`.
- `CXCodeCompleteResults` has a `results` field (which is currently found on 
Python bindings side in `CCRStructure`) , which would be added to 
`CodeCompletionResults`, leading to a collision.

The current `results` property is obviously sort of pointless, but it is also 
used at the moment to access the actual `CodeCompletionResult` objects, so 
removing this (rather, replacing it with the C-field) would break things for 
users. In fact, this leads to a rather difficult to understand bug, as (some) 
uses of `CodeCompletionResults.results` would just result in segmentation 
faults.

As a result, I recommend we 
- add the operators to iterate over the `CodeCompletionResult` array in 
`CCRStructure` to `CodeCompletionResults` as well, in Clang 22
- add a deprecation warning to `CodeCompletionResults.results`, to tell users 
they don't need to go through it anymore since they can iterate over 
`CodeCompletionResult`s directly via `CodeCompletionResults`
- then in Clang 23 (together with all the other changes we are currently 
issuing deprecation warnings for) "deprecate" the `results` property, actually 
folding `CCRStructure` into `CodeCompletionResults`.

The only issue here is that `CodeCompletionResults.results` doesn't become 
unusable, it is just replaced by the C-field. Like I said above, this may lead 
to hard to spot bugs, but that's basically our only choice I believe.
The changes for Clang 22 would look like what you see in this draft PR
What do you think about this approach?

https://github.com/llvm/llvm-project/pull/177764
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to