[Clang] Fix dependency of SourceLocExpr.

[cor3ntin]

SourceLocExpr that may produce a function name
are marked dependent so that the non-instantiated
name of a function does not get evaluated.

In GH78128, the name('s size) is used as
template argument to a DeclRef that is not otherwise
dependent, and therefore cached and not transformed when the function is instantiated, leading
to 2 different values existing at the same time for the same function.

Fixes #78128

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

SourceLocExpr that may produce a function name
are marked dependent so that the non-instantiated
name of a function does not get evaluated.

In GH78128, the name('s size) is used as
template argument to a DeclRef that is not otherwise
dependent, and therefore cached and not transformed when the function is instantiated, leading
to 2 different values existing at the same time for the same function.

Fixes #78128

---

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

5 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+4)
• (modified) clang/include/clang/AST/Expr.h (+11)
• (modified) clang/lib/AST/Expr.cpp (+4-1)
• (modified) clang/lib/Sema/TreeTransform.h (+1-1)
• (modified) clang/test/SemaCXX/source_location.cpp (+27)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 174392da17551ea..a18596097ecd4df 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -913,6 +913,10 @@ Bug Fixes to C++ Support
   (`#57410 <https://github.com/llvm/llvm-project/issues/57410>`_) and
   (`#76604 <https://github.com/llvm/llvm-project/issues/57410>`_)
 
+- Fix a bug where clang would produce inconsistent values when
+  std::source_location::current was used in function templates.
+  Fixes  (`#78128 <https://github.com/llvm/llvm-project/issues/78128>`_)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index a41f2d66b37b69d..8bce87fa7a46eb2 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4806,6 +4806,17 @@ class SourceLocExpr final : public Expr {
     return T->getStmtClass() == SourceLocExprClass;
   }
 
+  static bool MayBeDependent(SourceLocIdentKind Kind) {
+    switch (Kind) {
+    case SourceLocIdentKind::Function:
+    case SourceLocIdentKind::FuncSig:
+    case SourceLocIdentKind::SourceLocStruct:
+      return true;
+    default:
+      return false;
+    }
+  }
+
 private:
   friend class ASTStmtReader;
 };
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index a90f92d07f86d26..11697a07b0e3228 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2282,7 +2282,10 @@ SourceLocExpr::SourceLocExpr(const ASTContext &Ctx, SourceLocIdentKind Kind,
     : Expr(SourceLocExprClass, ResultTy, VK_PRValue, OK_Ordinary),
       BuiltinLoc(BLoc), RParenLoc(RParenLoc), ParentContext(ParentContext) {
   SourceLocExprBits.Kind = llvm::to_underlying(Kind);
-  setDependence(ExprDependence::None);
+  // In dependent contexts, function names may change.
+  setDependence(MayBeDependent(Kind) && ParentContext->isDependentContext()
+                    ? ExprDependence::Value
+                    : ExprDependence::None);
 }
 
 StringRef SourceLocExpr::getBuiltinStr() const {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1a1bc87d2b3203c..4463904b07211bd 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12148,7 +12148,7 @@ TreeTransform<Derived>::TransformCXXMemberCallExpr(CXXMemberCallExpr *E) {
 
 template <typename Derived>
 ExprResult TreeTransform<Derived>::TransformSourceLocExpr(SourceLocExpr *E) {
-  bool NeedRebuildFunc = E->getIdentKind() == SourceLocIdentKind::Function &&
+  bool NeedRebuildFunc = SourceLocExpr::MayBeDependent(E->getIdentKind()) &&
                          getSema().CurContext != E->getParentContext();
 
   if (!getDerived().AlwaysRebuild() && !NeedRebuildFunc)
diff --git a/clang/test/SemaCXX/source_location.cpp b/clang/test/SemaCXX/source_location.cpp
index e92fb35b653a8f3..7414fbce7828d15 100644
--- a/clang/test/SemaCXX/source_location.cpp
+++ b/clang/test/SemaCXX/source_location.cpp
@@ -805,3 +805,30 @@ static_assert(S(0).j == S{0}.j);
 static_assert(S(0).j == S{0}.i);
 }
 #endif
+
+namespace GH78128 {
+
+template<int N>
+constexpr int f() {
+  return N;
+}
+
+template<typename T>
+void foo() {
+  constexpr auto* F1 = std::source_location::current().function();
+  static_assert(__builtin_strlen(F1) == f<__builtin_strlen(F1)>());
+
+  constexpr auto* F2 = __builtin_FUNCTION();
+  static_assert(__builtin_strlen(F2) == f<__builtin_strlen(F2)>());
+
+#ifdef MS
+  constexpr auto* F3 = __builtin_FUNCSIG();
+  static_assert(__builtin_strlen(F3) == f<__builtin_strlen(F3)>());
+#endif
+}
+
+void test() {
+  foo<int>();
+}
+
+}

[AaronBallman]

LGTM aside from some nits with the release note.

[AaronBallman]

Suggested change

	std::source_location::current was used in function templates.
	``std::source_location::current()`` was used in a function template.

[AaronBallman]

Suggested change

	Fixes (`#78128 <https://github.com/llvm/llvm-project/issues/78128>`_)
	Fixes (`#78128 <https://github.com/llvm/llvm-project/issues/78128>`_)