Comment 1 Sergey Prigogin 2008-10-08 16:38:51 EDT

Please provide implementation and tests in form of a patch.

Comment 2 Missing name Mising name 2008-10-09 03:46:51 EDT

Created attachment 114641 [details]
un-tgz'd patch without illustrative test content

Comment 3 Markus Schorn 2008-10-09 05:09:39 EDT

Sebastian, thanks for looking into this. I have reviewed your patch and I think it needs to be improved. C++-friends are in many ways similar to base-classes. Therefore the modelling should follow what we have for base classes:
* use PDOMName.getEnclosingDefinition() to find the class containing the friend.
* use PDOMName.getBinding() to find the binding of the friend.
* the name that 'owns' the friend needs to be marked, such that the friend 
  relationship is removed when the name is gone:
  see PDOMCPPLinkage.onDeleteName(...).

Comment 4 Missing name Mising name 2008-10-09 11:12:55 EDT

Created attachment 114690 [details]
patch aligned with handling of PDOMCPPClassType.getBases

Thanks for the tips Markus. I created a PDOMCPPFriend class in the end - despite the fact there isn't an ICPPFriendBinding (or similar) - due to the design of the PDOMNodeLinkedList.

I haven't been able to test the PDOMCPPLinkage.onDeleteName as I don't have a test env to support this, but it mimics the base treatment.

s

Comment 5 Missing name Mising name 2008-10-09 11:14:34 EDT

Created attachment 114691 [details]
missing file from diff

Comment 6 Markus Schorn 2008-10-14 06:45:25 EDT

Thanks, I have reviewed the patch:
* Please use workspace patches (they'll contain added files as well), (see
  http://wiki.eclipse.org/CDT/contributing)
* The record size of the PDOMCPPFriend needs to take the size of PDOMNode into
  account.
* Because this fix changes the database layout the pdom-version needs to be 
  incremented and documented in file PDOM.java.
* We need at least two testcases:
  + it is easy to run and extend existing tests. Make sure you have the plugin
    org.eclipse.cdt.core.tests in your workspace. In the context menu on any
    test-suite you can select 'Run as Junit Plugin Test'
  + org.eclipse.cdt.internal.pdom.tests.ClassTests already contains the
    testcase _testFriend(). The underscore marks it as failing. With your 
    patch it should pass and the '_' needs to be removed.
  + In org.eclipse.cdt.internal.index.tests.IndexUpdateTests you can test
    adding and removing friends to/from a class definition. 
  
Other than that the patch looks great!

Comment 7 Missing name Mising name 2008-10-23 15:52:00 EDT

Created attachment 115977 [details]
patch as per suggestions

- PDOMCPPFriend record size now takes PDOMNode record size into acct
- PDOM version incremented
+ above

Hope this suits.
Cheers,
s

Comment 8 Missing name Mising name 2008-10-23 15:56:22 EDT

Created attachment 115979 [details]
test cases for friend handling

- reinstated org.eclipse.cdt.internal.pdom.tests.ClassTests.testFriends which now passes
- added 3 test cases to org.eclipse.cdt.internal.index.tests.IndexUpdateTests corresponding to simple confirmation that a friend binding exists, then confirmation of their removal for
   - friend class
   - friend member function
   - friend function

Comment 9 Markus Schorn 2008-10-24 10:20:58 EDT

Great job! I have committed your patches.

Provides ClassTests.testFriend(), IndexUpdateTests.testFriendFunction(),
fixed in 6.0 > 20081024.