[clang] source range of variable template specialization should include initializer

[tomasz-kaminski-sonarsource]

Summary

This patch adjust the getSourceRange() for the
VarTemplateSpecializationDecl and VarTemplatePartialSpecializationDecl,
such that the initializer is included if present:

template<typename T>
T temp = 1;

template<> double temp<double> = 1;

This patch makes it consistent with the behavior of
non-template variables with initializers and restores
behavior that was present before https://reviews.llvm.org/D139705.

n case, when the initializer is not present we still
include the template arguments in the source range,
which was required for fixing zero-initialization fix-it.

[erichkeane]

This needs a test and a release note. Patch otherwise looks fine to me.

[tomasz-kaminski-sonarsource]

Comments no longer misleading reffer to fallback to DeclatorDecl

[erichkeane]

clang/lib/AST/DeclTemplate.cpp

1460

Looking closer... VarDecl already includes its init in its source range, right? So perhaps all that needs to happen here is to have the isExplicitSpecialization check be :

isExplicitSpecialization() && !hasInit()

WDYT?

[tomasz-kaminski-sonarsource]

In D146733#4216931, @erichkeane wrote:

This needs a test and a release note. Patch otherwise looks fine to me.

Do you have any pointers for a test of source ranges? The patch that caused regression used a fix-it hit, however, it is not useful in this case, as not fixes are raised in the case when the initializer is present.

[erichkeane]

Just noticed your commit-notice reference to the 'breaking' change is incorrect, should be:
https://reviews.llvm.org/D139705

As for the test, an AST-Dump test has the line/column info in it, so that should be a good enough test.

[tomasz-kaminski-sonarsource]

Simpified condition

[tomasz-kaminski-sonarsource]

I have updated the description and implementation. Will look at adding a unit test tomorrow.

clang/lib/AST/DeclTemplate.cpp

1460

Great suggestion. I really disliked the repetition.

[tomasz-kaminski-sonarsource]

Adding tests.

[erichkeane]

Test looks fine, still need a release note.

[tomasz-kaminski-sonarsource]

Including release note

[erichkeane]

clang/docs/ReleaseNotes.rst

231

Needs a little more detail here. Something about what was wrong before, and what is wrong now, particularly since there is no github bug to link to.

[tomasz-kaminski-sonarsource]

Updated release note to be more descriptive.

[erichkeane]

LGTM! Let me know if you need someone to commit this for you, and include "Name To Use <EmailAddr>"

[tomasz-kaminski-sonarsource]

Rebasing on test commit. My applogies for noise, I still get confused with arc sometimes.

[tomasz-kaminski-sonarsource]

In D146733#4219448, @erichkeane wrote:

LGTM! Let me know if you need someone to commit this for you, and include "Name To Use <EmailAddr>"

I have commit rights, but I will not be able to commit this before Monday.
I someone would like to commit these two patches, which I would appreciate, please use "Tomasz Kamiński <[e-mail]>".

clang/docs/ReleaseNotes.rst

- Fix crash when using ``[[clang::always_inline]]`` or ``[[clang::noinline]]``
   statement attributes on a call to a template function in the body of a
   template function.
 - Fix coroutines issue where ``get_return_object()`` result was always eargerly
   converted to the return type. Eager initialization (allowing RVO) is now only
   perfomed when these types match, otherwise deferred initialization is used,
   enabling short-circuiting coroutines use cases. This fixes
   (`#56532 <https://github.com/llvm/llvm-project/issues/56532>`_) in
   antecipation of `CWG2563 <https://cplusplus.github.io/CWG/issues/2563.html>_`.
 - Fix highlighting issue with ``_Complex`` and initialization list with more than
   2 items. (`#61518 <https://github.com/llvm/llvm-project/issues/61518>`_)
+- Fix  ``getSourceRange`` on  ``VarTemplateSpecializationDecl`` and
+  ``VarTemplatePartialSpecializationDecl``, which represents variable with
+  the initializer, so it behaves consistently with other ``VarDecls`` and ends
+  on the last token of initializer, instead of right angle bracket of
+  the template argument list.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support

clang/include/clang/AST/DeclTemplate.h

void setTemplateKeywordLoc(SourceLocation Loc) {
     ExplicitInfo->TemplateKeywordLoc = Loc;
   }
 
   /// Gets the location of the template keyword, if present.
   SourceLocation getTemplateKeywordLoc() const {
     return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
   }
 
-  SourceRange getSourceRange() const override LLVM_READONLY {
-    if (isExplicitSpecialization()) {
-      if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
-        return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
-    }
-    return VarDecl::getSourceRange();
-  }
+  SourceRange getSourceRange() const override LLVM_READONLY;
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     Profile(ID, TemplateArgs->asArray(), getASTContext());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID,
                       ArrayRef<TemplateArgument> TemplateArgs,
                       ASTContext &Context) {
public:
   /// Note that this member template is a specialization.
   void setMemberSpecialization() {
     auto *First = cast<VarTemplatePartialSpecializationDecl>(getFirstDecl());
     assert(First->InstantiatedFromMember.getPointer() &&
            "Only member templates can be member template specializations");
     return First->InstantiatedFromMember.setInt(true);
   }
 
-  SourceRange getSourceRange() const override LLVM_READONLY {
-    if (isExplicitSpecialization()) {
-      if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
-        return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
-    }
-    return VarDecl::getSourceRange();
-  }
+  SourceRange getSourceRange() const override LLVM_READONLY;
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     Profile(ID, getTemplateArgs().asArray(), getTemplateParameters(),
             getASTContext());
   }
 
   static void
   Profile(llvm::FoldingSetNodeID &ID, ArrayRef<TemplateArgument> TemplateArgs,

clang/lib/AST/DeclTemplate.cpp

 }
 
 void VarTemplateSpecializationDecl::setTemplateArgsInfo(
     const ASTTemplateArgumentListInfo *ArgsInfo) {
   TemplateArgsInfo =
       ASTTemplateArgumentListInfo::Create(getASTContext(), ArgsInfo);
 }
 
+SourceRange VarTemplateSpecializationDecl::getSourceRange() const {
+  if (isExplicitSpecialization() && !hasInit()) {
+    if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsInfo())
+      return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+  }
+  return VarDecl::getSourceRange();
+}
+
+
 //===----------------------------------------------------------------------===//
 // VarTemplatePartialSpecializationDecl Implementation
 //===----------------------------------------------------------------------===//
 
 void VarTemplatePartialSpecializationDecl::anchor() {}
 
 VarTemplatePartialSpecializationDecl::VarTemplatePartialSpecializationDecl(
     ASTContext &Context, DeclContext *DC, SourceLocation StartLoc,

 }
 
 VarTemplatePartialSpecializationDecl *
 VarTemplatePartialSpecializationDecl::CreateDeserialized(ASTContext &C,
                                                          unsigned ID) {
   return new (C, ID) VarTemplatePartialSpecializationDecl(C);
 }
 
+SourceRange VarTemplatePartialSpecializationDecl::getSourceRange() const {
+  if (isExplicitSpecialization() && !hasInit()) {
+    if (const ASTTemplateArgumentListInfo *Info = getTemplateArgsAsWritten())
+      return SourceRange(getOuterLocStart(), Info->getRAngleLoc());
+  }
+  return VarDecl::getSourceRange();
+}
+
 static TemplateParameterList *
 createMakeIntegerSeqParameterList(const ASTContext &C, DeclContext *DC) {
   // typename T
   auto *T = TemplateTypeParmDecl::Create(
       C, DC, SourceLocation(), SourceLocation(), /*Depth=*/1, /*Position=*/0,
       /*Id=*/nullptr, /*Typename=*/true, /*ParameterPack=*/false,
       /*HasTypeConstraint=*/false);
   T->setImplicit(true);

clang/test/AST/ast-dump-template-decls.cpp

 // FIXME (#61680) - serializing and loading AST should not affect reported source range
 // DIRECT:     VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:16> col:5 unTempl 'int'
 // SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:5> col:5 unTempl 'int'
 // CHECK-NEXT: `-TemplateArgument type 'int'
 // CHECK-NEXT: `-BuiltinType 0x{{[^ ]*}} 'int'
 
 template<>
 float unTempl<float> = 1;
-// FIXME (#61680) - serializing and loading AST should not affect reported source range
-// DIRECT:     VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:20> col:7 unTempl 'float' cinit
-// SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:24> col:7 unTempl 'float' cinit
+// CHECK:      VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:24> col:7 unTempl 'float' cinit
 // CHECK-NEXT: |-TemplateArgument type 'float'
 // CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
 // CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <col:24> 'float' <IntegralToFloating>
 // CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <col:24> 'int' 1
 
 template<class T, class U>
 T binTempl = 1;
 // CHECK:      VarTemplateDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:14> col:3 binTempl

 // SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:5> col:5 binTempl 'int'
 // CHECK-NEXT: |-TemplateArgument type 'int'
 // CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int'
 // CHECK-NEXT: `-TemplateArgument type 'int'
 // CHECK-NEXT: `-BuiltinType 0x{{[^ ]*}} 'int'
 
 template<>
 float binTempl<float, float> = 1;
-// FIXME (#61680) - serializing and loading AST should not affect reported source range
-// DIRECT:    VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:28> col:7 binTempl 'float' cinit
-// SERIALIZED: VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:32> col:7 binTempl 'float' cinit
+// CHECK:     VarTemplateSpecializationDecl 0x{{[^ ]*}} <line:{{[0-9]+}}:1, line:{{[0-9]+}}:32> col:7 binTempl 'float' cinit
 // CHECK-NEXT: |-TemplateArgument type 'float'
 // CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
 // CHECK-NEXT: |-TemplateArgument type 'float'
 // CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'float'
 // CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <col:32> 'float' <IntegralToFloating>
 // CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <col:32> 'int' 1
 }