[Clang][Sema] Fix issue on requires expression with templated base class member function

[jcsxky]

Fix #84020
Skip checking implicit object parameter in the context of RequiresExprBodyDecl.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

---

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

1 Files Affected:

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

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8725b09f8546cf..31df62d06f1c05 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7729,7 +7729,8 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
   }
 
   if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(FDecl))
-    if (Method->isImplicitObjectMemberFunction())
+    if (!isa<RequiresExprBodyDecl>(CurContext) &&
+        Method->isImplicitObjectMemberFunction())
       return ExprError(Diag(LParenLoc, diag::err_member_call_without_object)
                        << Fn->getSourceRange() << 0);
 

[tbaederr]

Please add a more useful PR description, since that's useful for reviewers and ends up in the git log.

[jcsxky]

Please add a more useful PR description, since that's useful for reviewers and ends up in the git log.

Thanks for your remind! I am waiting for CI to finish and after that I will add the description. Maybe mark it with draft would be better.

[erichkeane]

I don't really understand the change, can you better explain why this is the solution here?

[jcsxky]

In require body, name resolving just lookup function name and don't check function parameter without function call(I think).

struct B {
    template <typename S>
    void foo(int);

    void bar();
};

template <typename T, typename S>
struct A : T {
    auto foo() {
        static_assert(requires { T::template foo<S>(); });
        static_assert(requires { T::bar(); });
    }
};

int main() {
    A<B, double> a;
    //a.foo(0);
}

After add a parameter to foo, this code also compiles correctly without calling.

[erichkeane]

I guess I don't see how 'implicit object member function' is what we want there? It seems what we really want is 'in the context of a requires clause', right?

[jcsxky]

Yeah, I think it's like this, just with the same name is enough.

[jcsxky]

@erichkeane I am still feeling that we have no need to check whether the method has a explicit object parameter in require clause. In type requirement, checking nested member is enough(name resolution and parameter checking has already completed) and we don't need object argument since it isn't a function call.

struct S {
  void foo() {}
};
void bar() {
  S::foo(); // need object parameter
  foo();    // name resolution failed

It is only doing a function call that object parameter is required.

[erichkeane]

I still dont think 'implictObjectMemberFunction' is the right thing here. Why does the fact that it is not a 'Explit Object Member Function' matter?

like:

struct S {
void foo(this S){}
};
void bar() {
S::foo();
};

??

Or static member functions? This solution just doesn't seem right to me, and your responses dont' make it more clear.

[jcsxky]

Do you mean that checking whether Method is an implicit object function is redundant? After I removed

llvm-project/clang/lib/Sema/SemaExpr.cpp

Lines 7723 to 7726 in 844b532

	if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(FDecl))
	if (Method->isImplicitObjectMemberFunction())
	return ExprError(Diag(LParenLoc, diag::err_member_call_without_object)
	<< Fn->getSourceRange() << 0);

These testcases failed

llvm-project/clang/test/CXX/drs/dr3xx.cpp

Lines 1084 to 1095 in 844b532

	namespace cwg364 { // cwg364: yes
	struct S {
	static void f(int);
	void f(char);
	};
	void g() {
	S::f('a');
	// expected-error@-1 {{call to non-static member function without an object argument}}
	S::f(0);
	}
	}

llvm-project/clang/test/SemaTemplate/instantiate-using-decl.cpp

Lines 150 to 168 in 844b532

	struct UnresolvedTemplateNames {
	template <typename> void maybe_static();
	#if __cplusplus < 201103L
	// expected-warning@+2 {{default template arguments for a function template are a C++11 extension}}
	#endif
	template <typename T, typename T::type = 0> static void maybe_static();
	template <typename T>
	void instance_method() { (void)maybe_static<T>(); }
	template <typename T>
	static void static_method() {
	// expected-error@+1 {{call to non-static member function without an object argument}}
	(void)maybe_static<T>();
	}
	};
	void force_instantiation(UnresolvedTemplateNames x) {
	x.instance_method<int>();
	UnresolvedTemplateNames::static_method<int>(); // expected-note {{requested here}}
	}

without diagnose. Or the checking shouldn't be placed at current position?

[jcsxky]

And if only checking whether Method is a static method, we failed on:

llvm-project/clang/test/CXX/drs/dr26xx.cpp

Lines 225 to 240 in ef9446b

	#if __cplusplus >= 202302L
	namespace cwg2687 { // cwg2687: 18
	struct S{
	void f(int);
	static void g(int);
	void h(this const S&, int);
	};
	void test() {
	(&S::f)(1);
	// since-cxx23-error@-1 {{called object type 'void (cwg2687::S::*)(int)' is not a function or function pointer}}
	(&S::g)(1);
	(&S::h)(S(), 1);
	}
	}
	#endif

Why does the fact that it is not a 'Explit Object Member Function' matter?
Because 'Explit Object Member Function' can be invoked with member pointer with an object as the parameter (like S::h). While object is required when invoked an 'implictObjectMemberFunction'.

[erichkeane]

Ah, I had missed (thanks github review tool!) that the 'ImplicitObjectMember' part already existed. Sorry for that.

[jcsxky]

Ah, I had missed (thanks github review tool!) that the 'ImplicitObjectMember' part already existed. Sorry for that.

Never mind! Partly because I didn't make it more clear due to my poor expression .