Index distinct typedef in header file

[i-garrison]

GDB sources contain private portable regex implementation which provides own regmatch_t typedef. If another project file references system regex.h which provides regmatch_t typedef too, indexer would not adapt that binding and instead reuse first one it encountered.

Later when that other file is parsed, attempt to resolve binding for ASTName referring to regmatch_t would fail because found PDOM binding is reachable only via private implementation header which is not included.

Fix this by adapting unique typedef name to make both typedefs availabe to lookup procedure which will then filter by reachable index fragment.

This change is related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=214146 Typedef in a .cc file affects other .cc files

I found very similar issue first in unrelated workspace where one project carry a header file and another one uses that same file via symlinks on the filesystem, resulting in lots of name resolution errors. To me it looks like the strategy of adapting global names for classes and other entities has it's limits but cannot suggest anything better yet.

[github-actions]

Test Results

  585 files  ± 0    585 suites  ±0   13m 52s ⏱️ +15s
9 917 tests + 2  9 893 ✅ + 2  24 💤 ±0  0 ❌ ±0 
9 952 runs  +19  9 928 ✅ +19  24 💤 ±0  0 ❌ ±0 

Results for commit ed4a71c. ± Comparison against base commit 898cb90.

♻️ This comment has been updated with latest results.

[jonahgraham]

Thanks for trying to tackle this @i-garrison - let me know when it is ready for review + merge.

If you need help resolving this, I'm not sure I can help, but maybe some others may have some ideas, so please ask and we'll see what we can do.

[i-garrison]

Hi @jonahgraham thanks for looking into this.

I think I'm stuck at the moment because I do not understand the role of NestedBindingsIndex. In my attempt here one of PDOMBinding objects is found in PDOMLinkage both via getBindingsViaCache() and via visitor of getNestedBindingsIndex(). This looks wrong but I cannot pin where the issue is, should be somewhere in storing parent/child relationship but not clear where exactly, or I must be missing something obvious.

[i-garrison]

I think I'm stuck at the moment because I do not understand the role of NestedBindingsIndex. In my attempt here one of PDOMBinding objects is found in PDOMLinkage both via getBindingsViaCache() and via visitor of getNestedBindingsIndex(). This looks wrong but I cannot pin where the issue is, should be somewhere in storing parent/child relationship but not clear where exactly, or I must be missing something obvious.

I think I got it right and in the unit test testUnnamedNamespace_283679 bindings for doNothing are correctly found both in NestedBindingsIndex (because it belongs to unnamed namespace) and via bindings cache (because of how unnamed namespace works, name is visible in file scope as a result) - so my implemented filtering for duplicates in these changes is indeed working as expected.

One issue I found with remaining couple of cases is due to my implementation of search for similar bindings in other index fragments. I used AbstractCompositeFactory.fragmentComparator to detect if binding from the other index fragment is indeed the same object. It did not worked well because it scans qualified name fragments from the end, comparing them, but then chokes on CppCallHierarchy.testTemplates not being able to tell template CT from specialization CT<char> etc. which leads to wrong references collected.

Now if I fix fragmentComparator to compare signatures (i.e., if available, via IPDOMOverloader interface, or just names if not) then parent CT fragments are correctly differentiated and CppCallHierarchy.testTemplates started to pass. Hope this change to fragmentComparator is going in the right direction.

[jonahgraham]

Hope this change to fragmentComparator is going in the right direction.

Your explanation seem reassuring - so it certainly sounds like you are going in the right direction.

[i-garrison]

One more change needed is to allow promiscuous binding resolution to find local bindings. this fixes include organizer test which wants a header with typedef name; I also added a test for using declaration for completeness.

[i-garrison]

Looks like last thing to fix here is to return bindings in order of lookup (and just deduplicate results) which fixed PDOM search test.

[i-garrison]

Hi @jonahgraham I'm a bit confused about which of JUnit variants is needed to run unit tests. IIRC a few indexer tests do not work under JUnit5 because single project / multi project variants either are not created or not working because if changes in JUnit5. I see transition to JUnit5 started some time ago, but what is the current state?

[jonahgraham]

JUnit5 can be used to run the whole suite - but it is a lot of work to update the tests themselves to JUnit5 API. I have updated tests to JUnit5 when I have had other work to do in that area and when I decide the effort required is worth it / the benefit from new JUnit5 features valuable enough.

IIRC a few indexer tests do not work under JUnit5 because single project / multi project variants

I am not aware of any tests having a problem with running with JUnit5 runner. If there is a specific test(s) that you see an issue with, please let me know and I can look closer.

For clarity I mean running with JUnit5 means the JUnit5 dropdown in the launch configuration. This picture is from TESTING.md

--

I'm reviewing the change now.

[jonahgraham]

Thank you @i-garrison

[i-garrison]

I am not aware of any tests having a problem with running with JUnit5 runner. If there is a specific test(s) that you see an issue with, please let me know and I can look closer.

For clarity I mean running with JUnit5 means the JUnit5 dropdown in the launch configuration. This picture is from TESTING.md

Yes, that's the window I'm using to change between JUnit5 and JUnit3. Now if I run parser tests under JUnit5 I only see 4699 unit tests are run. If I change to JUnit3 then there are 5768 tests run. Not sure what's the actual issue (could be my openjdk-21).

I vaguely recall that e.g. those inheriting from IndexBindingResolutionTestBase and adding test cases during setup phase are not actually found by JUnit5 because they do not match JUnit5 unit test class discovery expectations. I remember looking into this while working on IndexConcepTest which is most recent of this kind. I decided to switch to JUnit3 for parser tests locally for now and just ask about current state of JUnit3 vs JUnit5 so we probably can pick the right version and fix any tests which are not found or fail with that.