Community
Participate
Working Groups
Created attachment 194665 [details] Open Declaration, Declaration in macro patch Example: #define DECLARE_INT(a) int a; DECLARE_INT(test_int) int main() { test_int; return 0; } Open Declaration on the second 'test_int' using F3 or hyperlink. 'DECLARE_INT(test_int)' is selected instead of 'test_int'.
Created attachment 194791 [details] Open Declaration, Declaration in macro patch + test, partial fix This patch only fixes the case where the macro is in the same translation unit. Here's a better example of when this bug is really annoying Macrotest.h: #define DECLARE_INTS(...) int __VA_ARGS__; DECLARE_INTS( int1, int2, int3) Macrotest.cpp: int main() { int1; } Opening the declaration of int1 selects all 4 lines of the macro in the header. The code I work with has much bigger lists which makes Open Declaration very imprecise. I'm not sure how to handle the case where the declaration is in a different translation unit. It seems like PDOMName would need more information. Maybe it should keep the image location as well? or have a flag specifying that it's inside a macro so we'll know we need to build that AST to find the image location?
Markus, do you you have an opinion on the problem mentioned in my last comment? or any advice on how to workaround it?
(In reply to comment #2) > Markus, do you you have an opinion on the problem mentioned in my last comment? > or any advice on how to workaround it? Hi Marc-Andre, When the image-location of the name of a declaration is of kind ARGUMENT_TO_MACRO_EXPANSION, it is indeed desirable to use this location for navigations to the declaration. * In your patch you need to check the kind of image location, because navigating to image-locations of kind MACRO_DEFINITION would be confusing. To deal with the navigation into a different file there are two options: (a) In case it is a ARGUMENT_TO_MACRO_EXPANSION, store the image-location of the declaration in the index. (b) When performing the navigation, create the AST for the target file and test whether there is a image-location of kind ARGUMENT_TO_MACRO_EXPANSION. Variant (a) is more complete because it would also affect other clients of the index, e.g. the C/C++ search. However, tracking image locations can be expensive, both in terms of memory and CPU. That's why the feature is turned off when the indexer computes ASTs. Variant (b) requires the computation of an additional AST. The computation can be limited to those cases where the text under the location does not match the name of the declaration. Cannot be done for search results, because there are too many different ASTs involved.
Marc-André, I borrowed part of your patch here to fix bug 509733. Hope you don't mind :)
(In reply to Nathan Ridge from comment #4) > Marc-André, I borrowed part of your patch here to fix bug 509733. Hope you > don't mind :) I don't mind :) Did you take this comment into consideration? (In reply to Markus Schorn from comment #3) > * In your patch you need to check the kind of image location, because > navigating to image-locations of kind MACRO_DEFINITION would be confusing.
(In reply to Marc-André Laperle from comment #5) > Did you take this comment into consideration? > > (In reply to Markus Schorn from comment #3) > > * In your patch you need to check the kind of image location, because > > navigating to image-locations of kind MACRO_DEFINITION would be confusing. I did not - thanks for pointing that out! I will post a follow-up patch to adjust this.
