Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ Bug Fixes to C++ Support
object parameter.
Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), (#GH86398), and (#GH86399).
- Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
- Fix a crash in requires expression with templated base class member function. Fixes (#GH84020).

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7739,7 +7739,8 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
}

if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(FDecl))
if (Method->isImplicitObjectMemberFunction())
if (!isa<RequiresExprBodyDecl>(CurContext) &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@jcsxky jcsxky Apr 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@jcsxky jcsxky Apr 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.

Method->isImplicitObjectMemberFunction())
return ExprError(Diag(LParenLoc, diag::err_member_call_without_object)
<< Fn->getSourceRange() << 0);

Expand Down
23 changes: 23 additions & 0 deletions clang/test/SemaCXX/PR84020.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// RUN: %clang_cc1 -std=c++20 -verify %s
// RUN: %clang_cc1 -std=c++23 -verify %s
// expected-no-diagnostics

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

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();
}
Loading