Page MenuHomePhabricator

[Sema] Fix bug to do with lookup of template friend function in namespace
Needs ReviewPublic

Authored by erik.pilkington on May 11 2016, 5:58 PM.

Details

Reviewers
rsmith
Summary

Clang erroneously rejects the following code because it does not find the default argument for makeType :

namespace n {
  template <class> struct Type {
    template <class T> friend Type<T> makeType();
  };

  template <class T = void> Type<T> makeType();

  void f() {
    Type<void> t;
    n::makeType<>(); // Error, lookup finds wrong version of makeType
  }
} // end namespace n

The problem is that during instantiation of Type<void> on the previous line the default template argument version of makeType is incorrectly swapped out of DeclContext::LookupPtr for the friend version. This patch fixes that by disallowing the swap in NamedDecl::declarationReplaces().

This patch looks like it's a fix for PR10856, PR18038, PR20877, and PR26962.

Diff Detail

Event Timeline

erik.pilkington retitled this revision from to [Sema] Fix bug to do with lookup of template friend function in namespace.
erik.pilkington updated this object.
erik.pilkington added a reviewer: rsmith.
erik.pilkington added a subscriber: cfe-commits.

I see failure when running the Clang test suite with this patch:

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 80: template non-type parameter has a different type 'long' in template redeclaration
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 304: different type 'char' in template redeclaration
error: 'error' diagnostics seen but not expected: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 79: template non-type parameter has a different type 'long' in template redeclaration
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 303: template non-type parameter has a different type 'char' in template redeclaration
error: 'note' diagnostics expected but not seen: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 73: previous non-type template parameter with type 'int' is here
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 300: previous non-type template parameter with type 'int' is here
error: 'note' diagnostics seen but not expected: 
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 72: previous non-type template parameter with type 'int' is here
  File /Users/meadori/Code/src/llvm/llvm-mainline/tools/clang/test/SemaTemplate/friend-template.cpp Line 299: previous non-type template parameter with type 'int' is here
8 errors generated.

Very weird, the test suite runs fine on my machine. Looks like the expected line for the diagnostics on your run is always one more than the line of the actual diagnostic. Could you please update your build to top-of-trunk, reapply the patch, and run the suite again?
If that doesn't work, maybe trying running the test on this file:
https://gist.github.com/EPilkington/662b516d2448919aaeebe1e4dce9d92b

Thanks for pointing this out, and please let me know what happens!

Hmmm, it works on trunk r269671. Not sure what happened before.

rsmith edited edge metadata.May 23 2016, 1:31 PM

It sounds like we're failing to properly inherit default template arguments onto redeclarations generated by template instantiation, resulting in a violation of our AST invariants. This patch will hide the problem in some cases, but the right thing to do is to fix the root cause: we should call CheckTemplateParameterList to inherit the default template arguments when instantiating a function template declaration. (Note that TemplateDeclInstantiator::VisitClassTemplateDecl remembers to do this but TemplateDeclInstantiator::VisitFunctionTemplateDecl forgets -- as does our handling for variable templates, and for partial specializations of class templates and variable templates.)