Incorrect source range for instantiations of out-of-line defaulted special member functions of class templates

[cov-tprince]

Bugzilla Link	25683
Version	3.7
OS	All
CC	@synopsys-sig-compiler-frontends,@tahonermann

Extended Description

The following snippet produces an AST in which the starting source location appears after the end location:

$ cat bad.cpp
template struct s { s(); };
s a;
template s::s() = default;
$ clang -v -std=c++11 -Xclang -ast-dump -fsyntax-only bad.cpp
clang version 3.7.0 (tags/RELEASE_370/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6.3
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
"/opt/pkg/clang-3.7.0/bin/clang-3.7" -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -disable-free -main-file-name bad.cpp -mrelocation-model static -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -target-cpu x86-64 -v -dwarf-column-info -resource-dir /opt/pkg/clang-3.7.0/bin/../lib/clang/3.7.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/x86_64-linux-gnu -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/backward -internal-isystem /usr/local/include -internal-isystem /opt/pkg/clang-3.7.0/bin/../lib/clang/3.7.0/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -std=c++11 -fdeprecated-macro -fdebug-compilation-dir /slowfs/sighome/trprince/lab/bz82561 -ferror-limit 19 -fmessage-length 211 -mstackrealign -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -ast-dump -x c++ bad.cpp
clang -cc1 version 3.7.0 based upon LLVM 3.7.0 default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/x86_64-linux-gnu
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/backward
/usr/local/include
/opt/pkg/clang-3.7.0/bin/../lib/clang/3.7.0/include
/usr/include/x86_64-linux-gnu
/usr/include
End of search list.
TranslationUnitDecl 0x86c6510 <>
|-TypedefDecl 0x86c6a48 <> implicit __int128_t '__int128'
|-TypedefDecl 0x86c6aa8 <> implicit __uint128_t 'unsigned __int128'
|-TypedefDecl 0x86c6e88 <> implicit __builtin_va_list '__va_list_tag [1]'
|-ClassTemplateDecl 0x86c7020 <bad.cpp:1:1, col:37> col:28 s
| |-TemplateTypeParmDecl 0x86c6ed8 col:11 col:11 typename
| |-CXXRecordDecl 0x86c6f90 <col:21, col:37> col:28 struct s definition
| | |-CXXRecordDecl 0x870f060 <col:21, col:28> col:28 implicit referenced struct s
| | -CXXConstructorDecl 0x870f190 <col:32, col:34> col:32 s<type-parameter-0-0> 'void (void)' | -ClassTemplateSpecializationDecl 0x870f260 <col:1, col:37> col:28 struct s definition
| |-TemplateArgument type 'int'
| |-CXXRecordDecl 0x870f508 prev 0x870f260 <col:21, col:28> col:28 implicit struct s
| |-CXXConstructorDecl 0x870f5d8 <line:3:23, line:1:32> col:32 used constexpr s 'void (void) noexcept'
| | -CompoundStmt 0x870feb0 <col:32> | |-CXXConstructorDecl 0x870f6d8 <col:28> col:28 implicit constexpr s 'void (const struct s<int> &)' inline noexcept-unevaluated 0x870f6d8 | | -ParmVarDecl 0x870f820 col:28 col:28 'const struct s &'
| -CXXConstructorDecl 0x870f8b8 <col:28> col:28 implicit constexpr s 'void (struct s<int> &&)' inline noexcept-unevaluated 0x870f8b8 | -ParmVarDecl 0x870fa00 col:28 col:28 'struct s &&'
|-VarDecl 0x870f408 <line:2:1, col:8> col:8 a 's':'struct s' callinit
| -CXXConstructExpr 0x870fa68 <col:8> 's<int>':'struct s<int>' 'void (void) noexcept' -CXXConstructorDecl 0x870fd40 parent 0x86c6f90 prev 0x870f190 <line:3:1, col:31> col:29 s 'void (void)'
$

In particular, note the line numbers on the second CXXConstructorDecl:

CXXConstructorDecl 0x870f5d8 <line:3:23, line:1:32>

This doesn't seem to cause any problems in clang proper, but passing this range to clang::PreprocessingRecord::getPreprocessedEntitiesInRange triggers an assertion, since the start of the range occurs after the end.

[tahonermann]

I updated the summary as this issue occurs for all instantiated out-of-line defaulted special member function definitions, not just constructors. It does not occur for non-defaulted definitions (the following test case includes a member function for comparison purposes):

$ cat t.cpp
template<typename T>
struct S {
  S();
  S(const S&);
  S(S&&);
  ~S();
  S& operator=(const S&);
  S& operator=(S&&);
  void mf();
  T dm;
};
template<typename T>
S<T>::S() = default;
template<typename T>
S<T>::S(const S<T>&) = default;
template<typename T>
S<T>::S(S<T>&&) = default;
template<typename T>
S<T>::~S() = default;
template<typename T>
S<T>& S<T>::operator=(const S<T>&) = default;
template<typename T>
S<T>& S<T>::operator=(S<T>&&) = default;
template<typename T>
void S<T>::mf() {}
void instantiate_all_the_things() {
  S<int> s1;
  S<int> s2(s1);
  S<int> s3(static_cast<S<int>&&>(s2));
  s2 = s1;
  s3 = static_cast<S<int>&&>(s2);
  s3.mf();
}

$ clang --version
clang version 8.0.0 (tags/RELEASE_800/final)
Target: x86_64-unknown-linux-gnu
...

$ clang -c -Xclang -ast-dump t.cpp
TranslationUnitDecl 0x5efc378 <<invalid sloc>> <invalid sloc>
...
|-ClassTemplateDecl 0x5f38368 <t.cpp:1:1, line:11:1> line:2:8 S
...
| `-ClassTemplateSpecializationDecl 0x5f69b00 <line:1:1, line:11:1> line:2:8 struct S definition
...
|   |-CXXConstructorDecl 0x5f6a660 <line:13:1, line:3:5> col:3 used S 'void () noexcept' default
...
|   |-CXXConstructorDecl 0x5f6a828 <line:15:1, line:4:13> col:3 used constexpr S 'void (const S<int> &) noexcept' default
...
|   |-CXXConstructorDecl 0x5f6a9f8 <line:17:1, line:5:8> col:3 used constexpr S 'void (S<int> &&) noexcept' default
...
|   |-CXXDestructorDecl 0x5f6aad8 <line:19:1, line:6:6> col:3 used ~S 'void () noexcept' default
...
|   |-CXXMethodDecl 0x5f6acb8 <line:21:1, line:7:24> col:6 used operator= 'S<int> &(const S<int> &) noexcept' default
...
|   |-CXXMethodDecl 0x5f6ae38 <line:23:1, line:8:19> col:6 used operator= 'S<int> &(S<int> &&) noexcept' default
...
|   |-CXXMethodDecl 0x5f6aee8 <line:25:1, col:18> line:9:8 used mf 'void ()'
...

For each of the special member function declarations within the instantiated class template specialization, note that the begin location corresponds to the out-of-line definition, but that the end location corresponds to the in-class declaration. The instantiation of the 'mf' member function illustrates the expected behavior.

[tahonermann]

Debugging revelations from the following minimal test case:

template<typename>
struct S {
  S();
};
template<typename T>
S<T>::S() = default;
S<int> s;

With a break point set on clang::CXXConstructorDecl::Create(), a number of constructions of CXXConstructorDecl are observed. The first is for the in-class declaration at line 3. The second is for the out-of-line definition on lines 5-6. The third is for the instantiation of the constructor for the C specialization; the case this bug is concerned with.

That third instance is created here:

lib/Sema/SemaTemplateInstantiateDecl.cpp:
1903 Decl *
1904 TemplateDeclInstantiator::VisitCXXMethodDecl(CXXMethodDecl *D,
1905                                       TemplateParameterList *TemplateParams,
1906                                       bool IsClassScopeSpecialization) {
....
1983   SourceLocation StartLoc = D->getInnerLocStart();
....
1986   if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(D)) {
1987     Method = CXXConstructorDecl::Create(SemaRef.Context, Record,
1988                                         StartLoc, NameInfo, T, TInfo,
1989                                         Constructor->isExplicit(),
1990                                         Constructor->isInlineSpecified(),
1991                                         false, Constructor->isConstexpr());
1992     Method->setRangeEnd(Constructor->getEndLoc());
....
2196 }

Debugging revealed that 'D' corresponds to the first constructed CXXConstructorDecl object corresponding to the in-class declaration. Note that the provided StartLoc is obtained from getInnerLocStart() called for the in-class declaration. Since the in-class declaration does not have a template parameter list, this corresponds to the start location of that declaration, line 3:3. The end location is then set to the end location of that declaration at line 3:5, making the locations consistent.

What ends up happening is that a call to setInnerLocStart() is made here:

lib/Sema/SemaTemplateInstantiateDecl.cpp:
3890 void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
3891                                          FunctionDecl *Function,
3892                                          bool Recursive,
3893                                          bool DefinitionRequired,
3894                                          bool AtEndOfTU) {
....
4018   // Copy the inner loc start from the pattern.
4019   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
....
4100 }

Lines 4018-4019 above were added via the change in the following link to correct source locations for such instantiations.

• 12dcbf3

The call to setInnerLocStart() changes the perceived beginning of the function (since, for this declaration, there is no template parameter list). Within the debugger, calling Function->dump() before and after the call to setInnerLocStart() demonstrates the change:

(gdb) p Function->dump()
CXXConstructorDecl 0xae44e88 <m1.cpp:3:3, col:5> col:3 used S 'void ()'
...
(gdb) p Function->dump()
CXXConstructorDecl 0xae44e88 <m1.cpp:6:1, line:3:5> col:3 used S 'void ()'

The same inconsistent location issue occurs here for non-defaulted functions as well. However, for functions with bodies, the end location gets corrected later by a call to setBody() from Sema::ActOnFinishFunctionBody():

lib/AST/Decl.cpp:
2741 void FunctionDecl::setBody(Stmt *B) {
2742   Body = B;
2743   if (B)
2744     EndRangeLoc = B->getEndLoc();
2745 }

However, even in this case, the non-inner begin location is never updated with the result being:
Decl::Loc = begin location of the in-class declaration.
DeclaratorDecl::InnerLocStart = inner location of the out-of-line definition.
FunctionDecl::EndRangeLoc = end location of the out-of-line definition.

I'm not sure what the ramifications are of the begin and inner-begin locations being out of sync like this.

The following change appears to suffice to work around this issue:

$ git diff
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index e76a34c..a07742d 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4015,8 +4015,10 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   // unimported module.
   Function->setVisibleDespiteOwningModule();

-  // Copy the inner loc start from the pattern.
+  // Copy source locations from the pattern.
+  Function->setLocation(PatternDecl->getBeginLoc());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
+  Function->setRangeEnd(PatternDecl->getEndLoc());

   EnterExpressionEvaluationContext EvalContext(
       *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);

With the above change, I get 9 test failures in the Clang test suite. I haven't yet looked at the details of those failures.

Failing Tests (9):
    Clang :: Index/cursor-ref-names.cpp
    Clang :: Index/preamble_macro_template.cpp
    Clang :: Index/recursive-cxx-member-calls.cpp
    Clang :: SemaCXX/member-init.cpp
    Clang :: SemaTemplate/virtual-member-functions.cpp
    Clang :: Templight/templight-deduced-func.cpp
    Clang :: Templight/templight-default-func-arg.cpp
    Clang :: Templight/templight-exception-spec-func.cpp
    Clang :: Templight/templight-explicit-template-arg.cpp

  Expected Passes    : 42326
  Expected Failures  : 170
  Unsupported Tests  : 679
  Unexpected Failures: 9

[tahonermann]

A proposed fix has been posted for review:

• https://reviews.llvm.org/D64087

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[llvmbot]

@llvm/issue-subscribers-c-1

[Endilll]

I asked Tom to rebase and added Clang language WG as reviewers. Let's see if we can have some progress with this.