[Clang] FunctionEffects: properly extract the type of a bound member member function from a CallExpr.

[dougsonos]

There's a bug illustrated by this example:

template <typename T>
struct Holder {
	T value;
	
	T& operator*() { return value; }
};

struct X {
	using Dispatch = float (X::*)() [[clang::nonblocking]];
    
	void fails(Holder<Dispatch>& holder) [[clang::nonblocking]]
	{
		(this->*(*holder))();   <<< the expression is incorrectly determined not to be nonblocking
	}

	void succeeds(Holder<Dispatch>& holder) [[clang::nonblocking]]
	{
		auto func = *holder;
		(this->*func)();
	}
};

In both cases we have a CXXMemberCallExpr. In succeeds, the expression refers to a Decl (func) and gets a useful PTMF type. In fails, the expression does not refer to a Decl and its type is special, printed as bound member function. Expr provides a method for extracting the true type so we can use that in this situation.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

There's a bug illustrated by this example:

template <typename T>
struct Holder {
	T value;
	
	T& operator*() { return value; }
};

struct X {
	using Dispatch = float (X::*)() [[clang::nonblocking]];
    
	void fails(Holder<Dispatch>& holder) [[clang::nonblocking]]
	{
		(this->*(*holder))();   <<< the expression is incorrectly determined not to be nonblocking
	}

	void succeeds(Holder<Dispatch>& holder) [[clang::nonblocking]]
	{
		auto func = *holder;
		(this->*func)();
	}
};

In both cases we have a CXXMemberCallExpr. In succeeds, the expression refers to a Decl (func) and gets a useful PTMF type. In fails, the expression does not refer to a Decl and its type is special, printed as bound member function. Expr provides a method for extracting the true type so we can use that in this situation.

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaFunctionEffects.cpp (+11-2)
• (modified) clang/test/Sema/attr-nonblocking-constraints.cpp (+14-1)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 8590ee831084f..6d7bcbf53fe0f 100644
--- a/clang/lib/Sema/SemaFunctionEffects.cpp
+++ b/clang/lib/Sema/SemaFunctionEffects.cpp
@@ -1208,8 +1208,17 @@ class Analyzer {
         return true;
       }
 
-      // No Decl, just an Expr. Just check based on its type.
-      checkIndirectCall(Call, CalleeExpr->getType());
+      // No Decl, just an Expr. Just check based on its type. Bound member
+      // functions are a special expression type and need to be specially
+      // unpacked.
+      QualType CalleeExprQT = CalleeExpr->getType();
+      if (CalleeExpr->isBoundMemberFunction(Outer.S.getASTContext())) {
+        QualType QT = Expr::findBoundMemberType(CalleeExpr);
+        if (!QT.isNull()) {
+          CalleeExprQT = QT;
+        }
+      }
+      checkIndirectCall(Call, CalleeExprQT);
 
       return true;
     }
diff --git a/clang/test/Sema/attr-nonblocking-constraints.cpp b/clang/test/Sema/attr-nonblocking-constraints.cpp
index b26a945843696..22f7e23f81cf9 100644
--- a/clang/test/Sema/attr-nonblocking-constraints.cpp
+++ b/clang/test/Sema/attr-nonblocking-constraints.cpp
@@ -236,9 +236,22 @@ void nb13() [[clang::nonblocking]] { nb12(); }
 struct PTMFTester {
 	typedef void (PTMFTester::*ConvertFunction)() [[clang::nonblocking]];
 
+	ConvertFunction mConvertFunc;
+
 	void convert() [[clang::nonblocking]];
 
-	ConvertFunction mConvertFunc;
+	template <typename T>
+	struct Holder {
+		T value;
+		
+		T& operator*() { return value; }
+	};
+
+
+	void ptmfInExpr(Holder<ConvertFunction>& holder) [[clang::nonblocking]]
+	{
+		(this->*(*holder))(); // This should not generate a warning.
+	}
 };
 
 void PTMFTester::convert() [[clang::nonblocking]]

[Sirraide]

Some minor comments; lgtm otherwise

[Sirraide]

Can you also add

((*this).*(*holder))();

as well as a test case that shows that we do issue a warning if the member function is not nonblocking.