[clang][AST] Fix end location of DeclarationNameInfo on instantiated methods

[steakhal]

Fixes #71161

D64087 updated some locations of the instantiated method but forgot DNLoc.

FunctionDecl::getNameInfo() constructs a DeclarationNameInfo using Decl::Loc as the beginning of the declaration name, and FunctionDecl::DNLoc to compute the end of the declaration name. The former was updated, but the latter was not, so DeclarationName::getSourceRange() would return a range where the end of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166

[llvmbot]

@llvm/pr-subscribers-clang

Author: Balazs Benics (steakhal)

Changes

Fixes #71161

D64087 updated some locations of the instantiated method but forgot DNLoc.

FunctionDecl::getNameInfo() constructs a DeclarationNameInfo using Decl::Loc as the beginning of the declaration name, and FunctionDecl::DNLoc to compute the end of the declaration name. The former was updated, but the latter was not, so DeclarationName::getSourceRange() would return a range where the end of the declaration name could come before its beginning.

Patch by Alejandro Alvarez Ayllon
Co-authored-by: steakhal

CPP-5166

---

Full diff: https://github.com/llvm/llvm-project/pull/92654.diff

3 Files Affected:

• (modified) clang/include/clang/AST/Decl.h (+2)
• (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+1)
• (modified) clang/unittests/AST/DeclTest.cpp (+31)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 5e485ccb85a13..7fd80b90d1033 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2188,6 +2188,8 @@ class FunctionDecl : public DeclaratorDecl,
 
   void setRangeEnd(SourceLocation E) { EndRangeLoc = E; }
 
+  void setDeclarationNameLoc(DeclarationNameLoc L) { DNLoc = L; }
+
   /// Returns the location of the ellipsis of a variadic function.
   SourceLocation getEllipsisLoc() const {
     const auto *FPT = getType()->getAs<FunctionProtoType>();
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 381d79b2fcd46..6d736d0771eac 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5055,6 +5055,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
   Function->setRangeEnd(PatternDecl->getEndLoc());
+  Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
 
   EnterExpressionEvaluationContext EvalContext(
       *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index 2530ce74eb6a3..16aa2b50b7a06 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -545,3 +545,34 @@ TEST(Decl, TemplateArgumentDefaulted) {
   EXPECT_TRUE(ArgList.get(2).getIsDefaulted());
   EXPECT_TRUE(ArgList.get(3).getIsDefaulted());
 }
+
+TEST(Decl, CXXDestructorDeclsShouldHaveWellFormedNameInfoRanges) {
+  // GH71161
+  llvm::Annotations Code(R"cpp(
+template <typename T> struct Resource {
+  ~Resource(); // 1
+};
+template <typename T>
+Resource<T>::~Resource() {} // 2,3
+
+void instantiate_template() {
+  Resource<int> x;
+}
+)cpp");
+
+  auto AST = tooling::buildASTFromCode(Code.code());
+  ASTContext &Ctx = AST->getASTContext();
+
+  const auto &SM = Ctx.getSourceManager();
+  auto GetNameInfoRange = [&SM](const BoundNodes &Match) {
+    const auto *D = Match.getNodeAs<CXXDestructorDecl>("dtor");
+    return D->getNameInfo().getSourceRange().printToString(SM);
+  };
+
+  auto Matches = match(findAll(cxxDestructorDecl().bind("dtor")),
+                       *Ctx.getTranslationUnitDecl(), Ctx);
+  ASSERT_EQ(Matches.size(), 3U);
+  EXPECT_EQ(GetNameInfoRange(Matches[0]), "<input.cc:3:3, col:4>");
+  EXPECT_EQ(GetNameInfoRange(Matches[1]), "<input.cc:6:14, col:15>");
+  EXPECT_EQ(GetNameInfoRange(Matches[2]), "<input.cc:6:14, col:15>");
+}

[Endilll]

LGTM, but wait for more reviews.

[hokein]

nit: add a note in clang/docs/ReleaseNotes.rst

[steakhal]

nit: add a note in clang/docs/ReleaseNotes.rst

Thanks. Added. Let me know if it's in the right section. @hokein

[Abramo-Bagnara]

@alejandro-alvarez-sonarsource @steakhal

This breaks the type info loaded correctly in previous decl instantiation:

template <typename T>
struct s {
  operator T();
    return 0;
  }
};

void f() {
  s<int> x;
  (int)x;
}

In this case the instantiated conversion DeclarationNameInfo is reverted back to template parameter T, instead of correct SubstTemplateTypeParmType.

[steakhal]

@alejandro-alvarez-sonarsource @steakhal

This breaks the type info loaded correctly in previous decl instantiation: [...]

Thank you for reporting @Abramo-Bagnara. Could you create a dedicated GH issue for this please?
FYI: I don't have plans to work on it though, but someone else may would want to pick it up, thus a dedicated ticket would bring visibility and opportunity to someone to work on. Thank you.

[zyn0217]

@Abramo-Bagnara @steakhal I noticed the same issue and I have posted the fix in #134038