Comment 1 Doug Schaefer 2006-12-15 16:43:07 EST

I've agreed to take care of this. It'll be well into the new year before I get around to it, though.

Comment 2 Andrew Ferguson 2006-12-19 12:34:39 EST

Doug, do you have a feeling for what will go in the index for this?

I've started prototyping composite bindings for 74433 (this adds another implementation of the IBinding hierarchy so that composite bindings can be navigated between and constructed on demand), and am wondering what kind of impact to expect when support for templates arrives

(having had a quick look at the template binding hierarchy I've not seen anything that stands out as a problem)

Comment 3 Doug Schaefer 2006-12-19 12:56:12 EST

I haven't looked at it in detail, but I've generally matched the DOM binding hierarchy where possible. If you see no problem there then I don't anticipate any either.

Comment 4 Markus Schorn 2007-02-09 09:05:32 EST

*** Bug 157743 has been marked as a duplicate of this bug. ***

Comment 5 Bryan Wilkinson 2007-03-19 16:11:56 EDT

Created attachment 61328 [details]
initial patch

Created PDOM implementations for class templates, class partial specializations, function templates, template type parameters, class instances, function instances, and most specializations.

TODO:
- Base classes for class specializations and instances
- Function template specializations
- Method templates / instances / template specializations
- Constructor templates / instances / template specializations
- Template non-type parameters
- Template template parameters

Comment 6 Doug Schaefer 2007-03-19 17:06:57 EDT

the initial patch has been applied. We'll leave this open until were done. Great work Bryan! Markus if you want to look and see if what Bryan has done works for you, that would be great. I'll do some testing around here too.

Comment 7 Sergey Prigogin 2007-03-19 17:38:32 EDT

Could you please check if this helps with bug 175470. I'll take a look too.

Comment 8 Sergey Prigogin 2007-03-21 23:05:28 EDT

Unfortunately, the situation with bug 175470 hasn't improved yet.

Comment 9 Bryan Wilkinson 2007-04-03 14:18:27 EDT

Created attachment 62811 [details]
follow-up patch

- implements base classes for class instances and class specializations
- allows for instantiation/specialization during incremental reindexes
  - prior to this a full reindex was required to properly resolve newly added
    implicit instances and specializations
- renders templates less exception-prone
- cleaned up code from last patch

Andrew, I made some small changes to composite binding related code in:
- CompositeIndexBinding
- InternalTemplateInstantiatorUtil
- TemplateInstanceUtil
Can you verify these changes?

Comment 10 Doug Schaefer 2007-04-04 11:32:27 EDT

(In reply to comment #9)
Patch applied. Thanks Bryan and sorry for the delay :)

Comment 11 Andrew Ferguson 2007-04-04 11:56:58 EDT

> Andrew, I made some small changes to composite binding related code in:
> - CompositeIndexBinding
> - InternalTemplateInstantiatorUtil
> - TemplateInstanceUtil
> Can you verify these changes?

thanks, these look fine. I'm not sure why CompositeIndexBinding.equals is needed though?

One more general comment - is it correct to say that whether template instantiation creates a record in the PDOM depends on whether the corresponding template definition binding comes from the AST or the index?

Comment 12 Bryan Wilkinson 2007-04-04 12:09:32 EDT

CompositeIndexBinding.equals was needed so that the composite template parameters can be used as keys in ObjectMaps.  I also raised Bug #180784 which calls for an implementation of hashCode() for similar reasons.

If a template in the PDOM is being instantiated, the instance will be a PDOM binding if that binding already exists in the PDOM.  If the specific instance is not yet in the PDOM it can't be added, or else there will be a concurrent read/write, so instead a non-PDOM binding is created.  The next time there is an incremental or full reindex, the non-PDOM binding is then added to the PDOM.

Comment 13 Bryan Wilkinson 2007-04-16 09:48:56 EDT

Created attachment 63901 [details]
method and constructor templates

Created PDOM implementations for method and constructor templates/instances/template specializations, as well as typedef specializations.

TODO:
- Template non-type parameters
- Template template parameters

Comment 14 Doug Schaefer 2007-04-16 10:45:44 EDT

Comment on attachment 61328 [details]
initial patch

Patch has been applied.

Comment 15 Doug Schaefer 2007-04-16 10:46:06 EDT

Comment on attachment 62811 [details]
follow-up patch

Patch has been applied.

Comment 16 Doug Schaefer 2007-04-16 11:34:04 EDT

Comment on attachment 63901 [details]
method and constructor templates

Latest patch applied. Thanks, Bryan!

Comment 17 Andrew Ferguson 2007-04-19 11:51:49 EDT

I've been having a go at getting hold of a CPPConstructorTemplateSpecialization binding, in order to see how this works in the index in more detail. Putting a breakpoint on its constructor, and running the parser tests didn't show any being created. 

The code I thought might generate one was:

class A {};
class B {};

class D {
	template<typename T, typename X>
	D(T t, X x) { // CPPConstructorTemplate

	}

	template<>
	D<A,B>(A a, B b) { // would be a CPPConstructorTemplateSpecialization ?

	}

	template<typename T, typename X>
	void foo(T t, X x) { // CPPMethodTemplate

	}

	template<>
	void foo<A,B>(A a, B b) { // CPPMethodTemplateSpecialization

	}
};

where the second member declaration would generate the binding, but it looks like its not valid C++?

does anyone have any ideas when this binding type would be created?

thanks,
Andrew

Comment 18 Bryan Wilkinson 2007-04-19 13:20:28 EDT

Here are the binding types that should be created for a slightly altered version of your code.

class A {};
class B {};

class D {
        template<typename T, typename X>
        D(T t, X x) {} // CPPConstructorTemplate

        template<typename T, typename X>
        void foo(T t, X x) {} // CPPMethodTemplate
};

template<>
D::D(A a, B b) {} // CPPConstructorSpecialization

template<>
void D::foo<A,B>(A a, B b) {} // CPPMethodSpecialization

Note that these are *Specialization rather than *TemplateSpecialization as they have been fully specialized and are no longer templates.

This is my understanding as to how to create *TemplateSpecialization bindings:
Note: some of this doesn't yet work with the AST on its own so don't expect too much from index bindings

template<class K>
class D { //CPPClassTemplate
public:
	template<class T, class X>
	D(T t, X x) {} // CPPConstructorTemplate

	template<class T, class X>
	void foo(T t, X x) {} // CPPMethodTemplate
};

void bar() {
	D<int> *var = new D<int>(5, 6);
	// First D<int>: CPPClassInstance
	// Second D<int>: CPPConstructorInstance (there is no
	// CPPConstructorInstance class so this would be handled
	// like a CPPMethodInstance)
	// Now, getting the instance's specialized binding should
	// result in a CPPConstructorTemplateSpecialization
	var->foo<int,int>(7, 8);
	// foo -> CPPMethodTemplateSpecialization
	// foo<int,int> -> CPPMethodInstance
}

Comment 19 Bryan Wilkinson 2007-04-19 14:24:03 EDT

And on a related note I just found a bug which prevents the creation of index constructor template specializations and method template specializations.  I'll submit a patch as soon as I test it out.

Comment 20 Bryan Wilkinson 2007-04-19 17:53:13 EDT

Created attachment 64365 [details]
fix for constructor/method template specializations

Allows for the creation of PDOM constructor and method template specializations.

Comment 21 Andrew Ferguson 2007-04-20 11:12:41 EDT

(In reply to comment #18)
> Here are the binding types that should be created for a slightly altered
> version of your code.

thanks, this helps a lot :)

Comment 22 Doug Schaefer 2007-04-20 12:19:30 EDT

Comment on attachment 64365 [details]
fix for constructor/method template specializations

Patch applied, thanks Bryan!

Comment 23 Bryan Wilkinson 2007-04-23 16:11:15 EDT

Created attachment 64651 [details]
constructor instances

- Implemented the CPPConstructorInstance binding
- Implemented the PDOMCPPConstructorInstance binding
- Integrated these into binding resolution
- Test case AST2CPPSpecTest.test14_1s8() regressed
  * Implemented function to pointer conversion and array to pointer
    conversion for template non type parameters as described in 14.1
    Section 8 of the C++ specification

Andrew, if you are still interested, all of the names in the second code snippet in Comment #18 should now resolve to the correct bindings.

Comment 24 Doug Schaefer 2007-04-24 12:46:34 EDT

(In reply to comment #23)
> Created an attachment (id=64651) [details]
> constructor instances

Ooops, I waited too long and the patch doesn't apply anymore. Could you do an update and recreate the patch. Thanks.

Comment 25 Bryan Wilkinson 2007-04-24 13:17:45 EDT

Created attachment 64767 [details]
constructor instances (updated)

Updated patch.

Comment 26 Doug Schaefer 2007-04-24 13:39:40 EDT

Comment on attachment 64767 [details]
constructor instances (updated)

Patch applied. Thanks, Bryan!

Comment 27 Andrew Ferguson 2007-05-03 09:16:40 EDT

> Andrew, if you are still interested, all of the names in the second code
> snippet in Comment #18 should now resolve to the correct bindings.

thanks, I'm still interested :)

I've just updated the index resolution tests and made a minor fix as PDOMCPPConstructorInstance was returning the wrong node type. This is in org.eclipse.cdt.internal.index.tests.IndexCPPTemplateResolutionTest

Comment 28 Doug Schaefer 2007-05-03 09:45:38 EDT

Bryan is done as much as he's going to do, unless he fixes some bugs while he's back at school :).

Marking this done. We can raise new bugs on specific template features that aren't supported yet.