This is an archive of the discontinued LLVM Phabricator instance.

Fix instantiation of friend function templates
Needs ReviewPublic

Authored by sepavloff on Jun 27 2016, 1:07 PM.

Details

Summary

This change fixed function template instantiation if the template is
defined in a friend declaration inside a class template.

Instantiation of friend templates defined inline in general requires
knowledge of the class template specialization where the template body is
defined. For instance, in the code:

template<typename T> struct C {
    template<typename T1> friend void func(T1 *x) { T var; }
};
C<int> v;
void use(int *x) { func(x); }

To properly instantiate func it is necessary to know in which
particular specialization of the containing class C the function
template is defined, because its body depends on template parameter
of that class. However the pattern search made by
getTemplateInstantiationPattern end up with the definition in
template class and information about its specialization is lost.
Subsequent call to getTemplateInstantiationArgs does not have
actual information and cannot instantiate the function.

As a solution, this change adds additional argument to the function
getTemplateInstantiationPattern that is set to the class template
specialization if the function is defined in its friend declaration.
Similarly the function getTemplateInstantiationArgs gets additional
argument, which specifies this specialization.

This change fixes PR26512, PR19095 and PR34343.

Diff Detail

Event Timeline

sepavloff updated this revision to Diff 62008.Jun 27 2016, 1:07 PM
sepavloff retitled this revision from to Fix instantiation of friend function templates.
sepavloff updated this object.
sepavloff added a reviewer: rsmith.
sepavloff added a subscriber: cfe-commits.

Aligned implementation with D21508.

sepavloff updated this revision to Diff 107817.Jul 23 2017, 4:11 AM

Reworked patch

Used more general way to cope with calculation of instantiation
stack, which is suitable for cases represented in PR26512.

Added new tests.

sepavloff edited the summary of this revision. (Show Details)Jul 23 2017, 4:14 AM
sepavloff updated this revision to Diff 116484.Sep 24 2017, 7:31 AM
sepavloff edited the summary of this revision. (Show Details)

Added regression test for PR34343

Investigation of this defect is presented in https://bugs.llvm.org/show_bug.cgi?id=34343 .

sepavloff edited the summary of this revision. (Show Details)Sep 24 2017, 7:49 AM
sepavloff updated this revision to Diff 139758.Mar 25 2018, 8:25 PM

Rebased patch

Rebased the patch, it is still actual.

Shouldn't there just be a link in the AST from the instantiated FunctionTemplateDecl back to the original pattern? Maybe a generalization of InstantiatedFromMember in RedeclarablableTemplateDecl?

Shouldn't there just be a link in the AST from the instantiated FunctionTemplateDecl back to the original pattern? Maybe a generalization of InstantiatedFromMember in RedeclarablableTemplateDecl?

The field InstantiatedFromMember indeed helps finding function template definition, this patch only modifies search algorithm to make it suitable for friend templates. Friend function templates have dual nature, they may be members of redeclaration chains, like usual namespace level function templates and still use InstantiatedFromMember like member function templates. So the resulting search algorithm (implemented in getPatternFor) scans both redeclaration chain and the chain formed by InstantiatedFromMember.

Oh, I see. The code currently tries to work with just the specialization and the pattern. To do the instantiation, we have to find template arguments for the context in which the pattern appears. For function temploids that aren't defined in a friend declaration, we can just consider the semantic DC hierarchy of the specialization, which will be the same across all redeclarations. For function temploids that *are* defined in a friend declaration, we have to look at the lexical DC of the declaration that was actually instantiated from the class temploid that defined the friend function, which isn't necessarily the specialization because it might have been redeclared after the friend function was instantiated. So your patch is mostly just changing the find-the-pattern lookup to remember that lexical DC when we find a pattern this way, which seems sensible. Richard should look over the actual lookup-algorithm changes.

include/clang/AST/Decl.h
2443

"If this is non-null and the pattern is a friend function declaration, this is set to the class containing the friend declaration."

lib/AST/Decl.cpp
3389

I don't think this can fail; a friend should always be lexically nested in a class.

3399

It's unfortunate that this needs to walk the redeclaration chain itself looking for definitions. Can't you just call getDefinition to find the definition and then run your extra checks on that?

3444

Same observation.