Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the main part of the xref interface #504

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ikirill
Copy link

@ikirill ikirill commented Oct 29, 2018

I left for a while and completely forgot #384.

For the xref interface see https://github.com/emacs-mirror/emacs/blob/master/lisp/progmodes/xref.el, and the lisp xref backend in https://github.com/emacs-mirror/emacs/blob/master/lisp/progmodes/elisp-mode.el

This implements xref-find-{definitions,references}. It's not quite perfect

  • it will jump into system headers (not sure if this is good or not, I've been using it that way myself)
  • it will resolve a call of an overloaded function to the right definition
  • The find-references functionality seems a little wonky: it seems to find references to the thing at point only in the same file plus the file in which it is defined (?). Maybe I got the AST traversal logic wrong in Irony::xrefReferences, but I'm not sure.

There are several FIXME's left in the code, which may or may not be issues (please have a look), they were things I wasn't quite sure about.

I tested this in my own code, it seems to work, but I don't have any actual unit tests for it.

@ikirill
Copy link
Author

ikirill commented Oct 29, 2018

xref-apropos is missing because it is supposed to search for functions by string matching, and I couldn't find a way to make that into something meaningful specifically in C++.

Copy link
Owner

@Sarcasm Sarcasm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments, mostly cosmetic.
Code looks good, I will have to test.

Thanks for looking into this!

@@ -426,6 +428,7 @@ If no such file exists on the filesystem the special file '-' is
(display-warning 'irony "Performance will be bad because a\
pipe delay is set for this platform (see variable\
`w32-pipe-read-delay')."))))
(irony-xref--enter)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be optional?
I think some people use irony for completion only, and sometime use rtags or something else for cross reference.

(irony-iotask-package-task irony--t-xref-references
(car line-column) (cdr line-column))))))
(cl-loop for item in result
do (message "%S" item))))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug?

@@ -14,6 +14,8 @@ include(CTest)
check_for_in_source_build()
release_as_default_build_type()

set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g -O0 -fsanitize=address -fsanitize=undefined -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-padded -Wno-missing-prototypes -Wno-shadow-field-in-constructor")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants