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

Add an isDefn field to the tags table in the database (and fix a warning) #24

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

Conversation

nrnrnr
Copy link

@nrnrnr nrnrnr commented Dec 7, 2015

This patch extends the sqlite database with a new field isDefn, which is populated by calling the libclang API function

unsigned clang_isCursorDefinition (CXCursor)

A potential issue with this patch is that the database is not versioned, and if the new server is run against an old database, things silently fail. I don't know if a return code from sqlite is not being checked or exactly what the issue might be.

Ideally an API version number would be stored in the database and if the server detects an old database, a new column would be added (and perhaps all the tags entries removed). Unfortunately I don't know nough sqlite to make this happen. But if you want to pursue it, I can ask for help.

I also don't know how to extend the internal unit tests to test for this case. But 'make test' continues to pass, and I have eyeballed a database for my application and the results look good!

The patch also fixes a compiler warning.

If I knew more Cmake, I'd have it search all
versions from 3.2 to 3.5.  But I don't know
how to get Cmake to do that.
compiler had warned of signed/unsigned comparison
This patch extends the sqlite database with a new
field 'isDefn', which is populated by calling
the libclang API function

   unsigned clang_isCursorDefinition (CXCursor)

A potential issue with this patch is that the database
is not versioned, and if the new server is run against
an old database, things silently fail.  I don't know
if a return code from sqlite is not being checked
or exactly what the issue might be.

Ideally an API version number would be stored in the
database and if the server detects an old database,
a new column would be added (and perhaps all the tags
entries removed).  Unfortunately I don't know enough sqlite
to make this happen.  But if you want to pursue it,
I can ask for help.

I also don't know how to extend the internal unit tests
to test for this case.  But 'make test' continues to pass,
and I have eyeballed a database for my application and the
results look good!
@ffevotte
Copy link
Owner

ffevotte commented Dec 8, 2015

Thanks!

I'll try your patch and merge it as soon as I can.

We should also think about the use of such a field. Is it destined to stay hidden in the database, or do you have an idea how the information could be brought to the user?

As for SQL schema changes between versions, you're right in that clang-tags should at least detect problems, so that users can delete the database and let it be rebuilt.
I'm not an expert about such things; I'll try to look how this is traditionnally done in modern projects.


_(Related to #23)_

@nrnrnr
Copy link
Author

nrnrnr commented Dec 8, 2015

In my project, here's how the information will be brought to the user: if you click on a use, you are linked to the declaration (isDecl == 1 && isDefn == 0). If you then click on the declaration, you are brought to the definition (isDefn == 1). For identifiers that have only a definition and no declaration, clicking on the use goes to the definition in one step.

I imagine M-. in emacs could work the same way, but my emacs skills are poor.

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