FunctionDefinition::definition always looks for FunctionDefinition based on the DeclarationId, but we don't need to do that if we already have a FunctionDefinition.
This could cause problems if we have multiple (same) definitions.
Details
- Reviewers
mwolff - Group Reviewers
KDevelop - Commits
- R32:3c51faa280ab: FunctionDefinition: only look for (new/other) function definition if we don't…
- create a small cmake (or whatever) project with 3 targets
- util1.c / util2.c / util3.c (same content):
#include <stdio.h> #include <stdlib.h> void configure() { printf("do stuff in %s\n", __FILE__); } int main(int argc, char *argv[]) { configure(); puts("Hello, UTIL!"); exit(EXIT_SUCCESS); }
- CMakeLists.txt:
add_executable(util1 util1.c) add_executable(util2 util2.c) add_executable(util3 util3.c)
- open util1.c
- open the outline and select "configure"
- EXPECTED: it should open the configure in util1.c (or the currently opened file)
- repeat with util2 and util3
- ACTUAL: only one of them works, the other two point to the wrong file.
Diff Detail
- Repository
- R32 KDevelop
- Lint
Lint Skipped - Unit
Unit Tests Skipped
I'd say this new code should go into FunctionDefinition::definition instead, no? I.e. check there whether decl is already a definition and if so return that directly?
otherwise +1 for the idea, I wonder why this hasn't been done/spotted before ;-)
looking at the code, it really is missing there. You'll need a const cast but that's OK
I assume you ran the tests (esp. for clang?) and it all keeps passing? if so, then +1
+2 if you could write a proper test for the case you explained in your commit message
Yes, it keeps passing.
But I really struggle in writing a proper test for this. I need big help here.
For starters I am even struggling with the correct test location, is it test_quickopen.cpp?
As different tests have different helpers and stuff.
I am guessing a (class) "TestProject" could help me but I could not find an easy on how to interact with the quickopen UI element and trigger it to open/activate a document?
So basically I am really lost here :\
the test would go into clang's test_duchain.cpp, create a method in there with the three TestFiles with the appropriate contents. Then parse them all and finally query the functions declarations and verify that the appropriate function definition is returned always.
please cleanup the test slightly and then push it directly (no need for another round of review)
thanks!
plugins/clang/tests/test_duchain.cpp | ||
---|---|---|
2118 | wrap it in a lambda and call it three times with the file as input argument |