[Clang] Fix handling of non-member functions in isNormalAssignmentOperator()

[smanna12]

This patch correctes the handling of non-member functions in the isNormalAssignmentOperator function within CheckExprLifetime.cpp.

The previous implementation incorrectly assumed that FunctionDecl is always a CXXMethodDecl, leading to potential null pointer dereferencing.

Change: - Correctly handle the case where FD is not a CXXMethodDecl by using FD->getParamDecl(0)->getType().

This fix ensures that the function correctly handles non-member assignment operators, such as:

struct S {}; void operator|=(S, S) {}

This change improves the robustness of the isNormalAssignmentOperator function by correctly identifying and handling different types of function declarations.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (smanna12)

Changes

This patch replaces dyn_cast<> with cast<> for CXXMethodDecl in isNormalAssignmentOperator() function, assuming FD is always a CXXMethodDecl. This change simplifies the code by removing the null check and relying on cast to assert the type.

---

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

1 Files Affected:

• (modified) clang/lib/Sema/CheckExprLifetime.cpp (+2-2)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index a1a402b4a2b530..e5ed9012fede51 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -482,8 +482,8 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) {
     if (RetT->isLValueReferenceType()) {
       ASTContext &Ctx = FD->getASTContext();
       QualType LHST;
-      auto *MD = dyn_cast<CXXMethodDecl>(FD);
-      if (MD && MD->isCXXInstanceMember())
+      auto *MD = cast<CXXMethodDecl>(FD);
+      if (MD->isCXXInstanceMember())
         LHST = Ctx.getLValueReferenceType(MD->getFunctionObjectParameterType());
       else
         LHST = MD->getParamDecl(0)->getType();

[cor3ntin]

Does this work with an explicit object member function?

[Sirraide]

Suggested change

	else
	LHST = MD->getParamDecl(0)->getType();
	else
	LHST = FD->getParamDecl(0)->getType();

I’m pretty sure this is a typo and is actually supposed to be this. I don’t think a cast instead of dyn_cast is right here because this is also a valid assignment operator, but it isn’t a CXXMethodDecl:

struct S {};
void operator|=(S, S) {}

@cor3ntin’s question as to whether this handles explicit object parameters correctly is also something we should investigate, but that seems like a separate issue.

[smanna12]

Thank you @Sirraide for reviews!

[smanna12]

@cor3ntin’s question as to whether this handles explicit object parameters correctly is also something we should investigate, but that seems like a separate issue.

Agreed. This is a separate issue.