[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

1466

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

1466

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

228 ↗

(On Diff #508077)

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/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 (const Expr *Init = getInit()) {
+    SourceLocation InitEnd = Init->getEndLoc();
+    // If Init is implicit, ignore its source range
+    if (InitEnd.isValid() && InitEnd != getLocation())
+      return SourceRange(getOuterLocStart(), InitEnd);
+  }
+  if (isExplicitSpecialization()) {
+    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 (const Expr *Init = getInit()) {
+    SourceLocation InitEnd = Init->getEndLoc();
+    // If Init is implicit, ignore its source range
+    if (InitEnd.isValid() && InitEnd != getLocation())
+      return SourceRange(getOuterLocStart(), InitEnd);
+  }
+  if (isExplicitSpecialization()) {
+    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);