This is an archive of the discontinued LLVM Phabricator instance.

Add a way to get the CXCursor that corresponds to a completion result.
AbandonedPublic

Authored by milianw on Oct 9 2015, 11:10 AM.

Details

Summary

The new clang_getCompletionCursor function returns the CXCursor stored
in AllocatedCXCodeCompleteResults for a completion result identified
by the index, i.e. offset into CXCodeCompleteResults.Results.

If the result cannot be represented by a CXCursor, a null cursor is
returned. The same happens when the index is out of bounds.

TODO: Build a CXCursor for Macros that have a IdentifierInfo in the
CodeCompletionResult. I do not yet know how to get my hands
on a MacroDefinitionRecord for that identifier, which I can
then pass to cxcursor::MakeCXCursor.

Diff Detail

Event Timeline

milianw updated this revision to Diff 36969.Oct 9 2015, 11:10 AM
milianw retitled this revision from to Add a way to get the CXCursor that corresponds to a completion result..
milianw updated this object.

any suggestion as to where put unit tests for this?

tools/libclang/CIndexCodeCompletion.cpp
548

note that this cannot be const, as Result.CreateCodeCompletionString is not const...

milianw updated this object.Oct 9 2015, 11:12 AM
klimek edited edge metadata.Oct 12 2015, 1:29 AM

Tests for libclang are usually done by:

  • exposing the calls of the function through c-index-test.c
  • adding a test/Index/some.c test that tests the behavior

Let me know if you have more questions!

Long time no see - sorry for the delay. I want to pick this up again. Would you say I should add the tests as separate tests (i.e. something like -code-completion-cursors-at), or should I rather output the cursor in addition when requesting -code-completion-at, and then extend all existing unit tests with the cursor information?

Thanks

If somebody could answer Milian's questions about tests, I would try to pick this up, I think the problem it solves is still very relevant.

Long time no see - sorry for the delay. I want to pick this up again. Would you say I should add the tests as separate tests (i.e. something like -code-completion-cursors-at), or should I rather output the cursor in addition when requesting -code-completion-at, and then extend all existing unit tests with the cursor information?

As it's a new function, I'd add a separate test.

Long time no see - sorry for the delay. I want to pick this up again. Would you say I should add the tests as separate tests (i.e. something like -code-completion-cursors-at), or should I rather output the cursor in addition when requesting -code-completion-at, and then extend all existing unit tests with the cursor information?

As it's a new function, I'd add a separate test.

OK, cool. But to get high coverage we'd probably need to duplicate a lot of of the existing tests. Anyhow, I'm find with either approach as long as we can move this forward. @svenbrauch, will you take care of this? It would probably be enough for now to get some basic testing in.

@klimek, or anyone else: do you happen to have any feedback on the TODO question in the code? I.e. how can we get a cursor for a macro?

Long time no see - sorry for the delay. I want to pick this up again. Would you say I should add the tests as separate tests (i.e. something like -code-completion-cursors-at), or should I rather output the cursor in addition when requesting -code-completion-at, and then extend all existing unit tests with the cursor information?

As it's a new function, I'd add a separate test.

OK, cool. But to get high coverage we'd probably need to duplicate a lot of of the existing tests. Anyhow, I'm find with either approach as long as we can move this forward. @svenbrauch, will you take care of this? It would probably be enough for now to get some basic testing in.

If the alternative is to duplicate all the tests, then yes, I'd also prefer the other option :)

@klimek, or anyone else: do you happen to have any feedback on the TODO question in the code? I.e. how can we get a cursor for a macro?

Nope, I'd need to drill through the code to figure out whether it's possible, too :)

I will try to take care of this, yes. Thanks for the advice!