Skip to content
This repository has been archived by the owner on Jul 30, 2020. It is now read-only.

Annotate references with the name of the function in which they occur #292

Closed
HighCommander4 opened this issue Jan 12, 2018 · 13 comments
Closed

Comments

@HighCommander4
Copy link
Contributor

When performing Find References, it would be really useful if each reference was annotated with the name of the functin in which the reference occurs (in addition to the name of the file, which is already there).

@jacobdufault
Copy link
Owner

Can you share a screenshot? This is likely a vscode issue that cannot be fixed by cquery.

@HighCommander4
Copy link
Contributor Author

Sure. Given this code:

void target();

void function1() {
  target();
}

void function2() {
  target();
}

when you perform "Find References" on target, and, say, the reference in function1() is selected:

cfviz-refs

I would like to see function1 either at the top beside "test.cpp - 3 references", or in the side panel next to the entry being viewed (in which case, it can be there even when the entry is not selected).

(In this case, you can see function1 in the code preview, but imagine it was a longer function so you couldn't see its name without scrolling up, potentially a lot.)

@HighCommander4
Copy link
Contributor Author

I don't think it's a VSCode issue, because the server only sends the reference's character range:

    {
      "uri": "file:///home/nr/dev/projects/testing/test/test.cpp",
      "range": {
        "start": {
          "line": 3,
          "character": 2
        },
        "end": {
          "line": 3,
          "character": 8
        }
      }
    },

To implement this feature, the server would also need to send the name of the enclosing function for each reference.

@HighCommander4
Copy link
Contributor Author

As a point of comparison, consider the search results pane in Eclipse CDT:

cfviz-refs-eclipse

There is no preview window, just a copy of the line containing the reference, but the search result is annotated with the name of the function it's contained in.

@MaskRay
Copy link
Contributor

MaskRay commented Jan 14, 2018

interface ReferenceParams extends TextDocumentPositionParams { returns Location[] | null. I think TypeScript interfaces can be arbitrarily inherited so we can add extra fields to Location. But this requires UI support from language clients.

What is needed to be done on cquery's side:

  • Extend struct VarDefDefinitionData to add semantic container (a named container)
  • Support serialization of subclasses. This requires some pointer or std::unique_ptr to utilize virtual functions, or use ad-hoc type dispatching.

@jacobdufault
Copy link
Owner

Why can't we just add the information to TextDocumentPositionParams (or some derived type that the references message uses)? This approach eliminates the need for subclass serialization.

@HighCommander4
Copy link
Contributor Author

HighCommander4 commented Feb 7, 2018

Why can't we just add the information to TextDocumentPositionParams (or some derived type that the references message uses)? This approach eliminates the need for subclass serialization.

TextDocumentPositionParams is what the request message uses. We need a place to put the information in the response message, whose type is just Location[] | null.

@MaskRay
Copy link
Contributor

MaskRay commented Feb 7, 2018

this requires more info in QueryLocation and uses. I recently changed Id from size_t to uint32_t

// 12 bytes
struct QueryLocation {
  QueryFileId path; // 4 bytes
  Range range; // 8 bytes
};

struct QueryVar {
...
    std::vector<QueryLocation> uses;
};

We should think how to encode the parent symbol information without bloating the size too much.

struct IndexVar {
  ...
  std::vector<Range> uses;
};

@HighCommander4
Copy link
Contributor Author

HighCommander4 commented Feb 7, 2018

It looks like the necessary information is available from libclang via CXIdxEntityRefInfo::container. Currently, we only use it for functions, to populate IndexFuncRef::id inside IndexFunc::callers. To implement this fully, we will need to store the container for variable and type references as well.

@MaskRay
Copy link
Contributor

MaskRay commented Feb 7, 2018

For a symbol occurrence (uses variables in query.h and indexer.h), what we need to record:

  • Range range;
  • Maybe<Id<void>> parent; container information
  • QueryFileId path;
  • SymbolKind parent_kind for a variable, the container may be either a type or a function. there should be someway to differentiate them.
  • CXSymbolRole read/write/call ...

QueryFileId path; looks to me the most useless one, but we may need to encode the file information in Query{Func,Type,Var} so that we can get the file associate with a uses entry by looking up its Query{Func,Type,Var} entry.

@HighCommander4
Copy link
Contributor Author

I filed an LSP spec issue for this: microsoft/language-server-protocol#396.

@MaskRay
Copy link
Contributor

MaskRay commented Feb 11, 2018

Renamed {Index,Query}File::callers to uses because the uses can also be f = &foo;

360a4ca added config->extension.referenceContainer
but someone should turn all

  Maybe<typename F::Range> definition_spelling;
  Maybe<typename F::Range> definition_extent;

to
Use (because Use records the lexical parent information)

@MaskRay
Copy link
Contributor

MaskRay commented Feb 19, 2018

The uses of Query{Func,Type,Var} have been unified to std::vector<Use> uses;

struct IndexFunc {
...
  std::vector<Use> uses;
};

Use::kind Use::id represents the lexical parent.

@MaskRay MaskRay closed this as completed Feb 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants