[clang] Correct source locations for instantiations of out-of-line defaulted special member functions. (PR25683)

[tahonermann]

Summary

[clang] Correct source locations for instantiations of function templates.

This change corrects some cases where the source location for an instantiated specialization of a function template or a member function of a class template was assigned the location of a non-defining declaration rather than the location of the definition the specialization was instantiated from.

Fixes https://github.com/llvm/llvm-project/issues/26057

[tahonermann]

See PR25683 (https://bugs.llvm.org/show_bug.cgi?id=25683) for more details. The patch posted here differs slightly from what is posted in the PR; getLocation() is called instead of getBeginLoc() since the latter may return a customized begin location.

I believe the impact to the two tests is a desirable change; the instantiation note seems like it should be correlated with the definition, not the declaration.

[Endill]

@tahonermann Do you mind rebasing this on top of trunk if it's necessary?
Might be a good idea to resubmit this as PR.

[cor3ntin]

LGTM (assuming rebase goes well)
@tahonermann ping folks in the future, and apologies for taking... 4 years!

[tahonermann]

Rebased patch uploaded. This retains the same code change, but includes additional test updates for tests added since the first patch was submitted, as well as an additional patch to exercise a test case from https://github.com/llvm/llvm-project/issues/26057.

The changes made effectively complete changes originally made in commit 12dcbf3eaa6d2c8b9ee814ddb8bf23bef644bfaf (Fixed implicit instantiations source range.)

[tahonermann]

@Endill, thank you for the reminder about this old patch!

@cor3ntin, I added an additional test and updated a few other recently (relative to the original patch!) added tests. Would you be so kind as to give this another quick review?

[tahonermann]

Moved the added release note to the correct section.

[tahonermann]

@cor3ntin, any concerns or suggestions per my recent updates? I'll plan to land this in the next couple of days otherwise.

[cor3ntin]

1 nit but still LGTM

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

4993–4994

[tahonermann]

Addressed review feedback.

[steakhal]

This patch appears to introduce a bug in some source ranges.
Reported the regression at https://github.com/llvm/llvm-project/issues/71161.

clang/docs/ReleaseNotes.rst

 - Clang now properly handles out of line template specializations when there is
   a non-template inner-class between the function and the class template.
   (`#65810 <https://github.com/llvm/llvm-project/issues/65810>`_)
 
 - Clang now properly converts static lambda call operator to function
   pointers on win32.
   (`#62594 <https://github.com/llvm/llvm-project/issues/62594>`_)
 
+- Fixed some cases where the source location for an instantiated specialization
+  of a function template or a member function of a class template was assigned
+  the location of a non-defining declaration rather than the location of the
+  definition the specialization was instantiated from.
+  (`#26057 <https://github.com/llvm/llvm-project/issues/26057>`_`)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
   `Issue 64169 <https://github.com/llvm/llvm-project/issues/64169>`_
 - Remove unnecessary RecordLayout computation when importing UnaryOperator. The
   computed RecordLayout is incorrect if fields are not completely imported and
   should not be cached.
   `Issue 64170 <https://github.com/llvm/llvm-project/issues/64170>`_

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

if (Inst.isInvalid() || Inst.isAlreadyInstantiating())
     return;
   PrettyDeclStackTraceEntry CrashInfo(Context, Function, SourceLocation(),
                                       "instantiating function definition");
 
   // The instantiation is visible here, even if it was first declared in an
   // unimported module.
   Function->setVisibleDespiteOwningModule();
 
-  // Copy the inner loc start from the pattern.
+  // Copy the source locations from the pattern.
+  Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
+  Function->setRangeEnd(PatternDecl->getEndLoc());
 
   EnterExpressionEvaluationContext EvalContext(
       *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
 
   // Introduce a new scope where local variable instantiations will be
   // recorded, unless we're actually a member function within a local
   // class, in which case we need to merge our results with the parent
   // scope (of the enclosing function). The exception is instantiating

clang/test/CXX/class/class.compare/class.compare.default/p1.cpp

 enum e {};
 bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}}
 
 bool operator==(e *, int *) = default; // expected-error{{must have at least one}}
 } // namespace p2085
 
 namespace p2085_2 {
 template <class T> struct S6 {
-  // expected-error@+2{{found 'const int &'}}
-  // expected-error@+1{{found 'const float &'}}
   bool operator==(T const &) const;
 };
+// expected-error@+2{{found 'const int &'}}
+// expected-error@+1{{found 'const float &'}}
 template <class T> bool S6<T>::operator==(T const &) const = default;
 
 template struct S6<int>; // expected-note{{S6<int>::operator==' requested}}
 
 void f1() {
   S6<float> a;
   (void)(a == 0); // expected-note{{S6<float>::operator==' requested}}
 }

clang/test/SemaCXX/cxx2b-consteval-propagate.cpp

 template <typename T>
 constexpr int f(T t) {
     return id(0);
 }
 }
 
 namespace forward_declare_consteval{
 template <typename T>
-constexpr int f(T t);  // expected-note {{'f<int>' defined here}}
+constexpr int f(T t);
 
 auto a = &f<char>;
 auto b = &f<int>; // expected-error {{immediate function 'f<int>' used before it is defined}} \
                   // expected-note {{in instantiation of function template specialization}}
 
 template <typename T>
-constexpr int f(T t) {
+constexpr int f(T t) { // expected-note {{'f<int>' defined here}}
     return id(t); // expected-note {{'f<int>' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
 }
 }
 
 namespace constructors {
 consteval int f(int) {
   return 0;
 }

clang/test/SemaCXX/member-init.cpp

enum {
     NOE1 = noexcept(B<int>::C()), // expected-note {{in evaluation of exception spec}}
     NOE2 = noexcept(B<int>::D<int>()) // expected-note {{in evaluation of exception spec}}
   };
 };
 }
 
 namespace explicit_instantiation {
 template<typename T> struct X {
-  X(); // expected-note {{in instantiation of default member initializer 'explicit_instantiation::X<float>::n' requested here}}
+  X();
   int n = T::error; // expected-error {{type 'float' cannot be used prior to '::' because it has no members}}
 };
 template struct X<int>; // ok
-template<typename T> X<T>::X() {}
+template<typename T> X<T>::X() {} // expected-note {{in instantiation of default member initializer 'explicit_instantiation::X<float>::n' requested here}}
 template struct X<float>; // expected-note {{in instantiation of member function 'explicit_instantiation::X<float>::X' requested here}}
 }
 
 namespace local_class {
 template<typename T> void f() {
   struct X { // expected-note {{in instantiation of default member initializer 'local_class::f()::X::n' requested here}}
     int n = T::error; // expected-error {{type 'int' cannot be used prior to '::' because it has no members}}
   };

 template <class T>
 void foo(T v) {
   struct s {
     T *s = 0;
   };
 }
 template void foo(int);
 }
+
+namespace GH26057 {
+template<typename T>
+struct S {
+  S();
+  int dm = T::error; // expected-error {{type 'int' cannot be used prior to '::' because it has no members}}
+};
+template<typename T>
+S<T>::S() = default; // expected-note {{in instantiation of default member initializer 'GH26057::S<int>::dm' requested here}} \
+                     // expected-note {{in evaluation of exception specification for 'GH26057::S<int>::S' needed here}}
+template struct S<int>; // expected-note {{in instantiation of member function 'GH26057::S<int>::S' requested here}}
+}

clang/test/SemaTemplate/virtual-member-functions.cpp

 // RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++11 %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -DMSABI -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -DMSABI -fsyntax-only -std=c++98 -verify %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -DMSABI -fsyntax-only -std=c++11 -verify %s
 
 namespace PR5557 {
 template <class T> struct A {
-  A(); // expected-note{{instantiation}}
+  A();
   virtual int a(T x);
 };
-template<class T> A<T>::A() {}
+template<class T> A<T>::A() {} // expected-note{{instantiation}}
 
 template<class T> int A<T>::a(T x) { 
   return *x; // expected-error{{requires pointer operand}}
 }
 
 void f() {
   A<int> x; // expected-note{{instantiation}}
 }
 
 template<typename T>
 struct X {
   virtual void f();
 };
 
 template<>
 void X<int>::f() { }
 }
 
 // Like PR5557, but with a defined destructor instead of a defined constructor.
 namespace PR5557_dtor {
 template <class T> struct A {
   A(); // Don't have an implicit constructor.
-  ~A(); // expected-note{{instantiation}}
+  ~A();
   virtual int a(T x);
 };
-template<class T> A<T>::~A() {}
+template<class T> A<T>::~A() {} // expected-note{{instantiation}}
 
 template<class T> int A<T>::a(T x) { 
   return *x; // expected-error{{requires pointer operand}}
 }
 
 void f() {
   A<int> x; // expected-note{{instantiation}}
 }