Skip to content
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.

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

Use reflect_invoke metafunction to invoke a member function #79

Merged
merged 24 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d2b3a59
initial implementation
BaLiKfromUA Jul 23, 2024
cda6611
Merge branch 'p2996' into clang-p2996/issues/5-refined
BaLiKfromUA Jul 31, 2024
618dcf4
format
BaLiKfromUA Jul 31, 2024
cd8adb6
refactoring to simplify if statements
BaLiKfromUA Aug 1, 2024
e7899d2
support of template member functions
BaLiKfromUA Aug 1, 2024
dc8003c
add diagnostic
BaLiKfromUA Aug 2, 2024
81a4dd7
wording
BaLiKfromUA Aug 2, 2024
a8b7a4a
more optimal exclusion of object argument
BaLiKfromUA Aug 9, 2024
8d32747
hack with wrapping decl reference into splice expr to prevent lookup …
BaLiKfromUA Aug 12, 2024
6479e09
fix bug + add automated tests for diagnostics
BaLiKfromUA Aug 13, 2024
16e63c7
comment
BaLiKfromUA Aug 13, 2024
22a2108
clean up
BaLiKfromUA Aug 21, 2024
c416cfb
test refactoring
BaLiKfromUA Aug 21, 2024
2dd9246
apply different small review remarks
BaLiKfromUA Aug 23, 2024
fe85f72
add support for using methods of base classes
BaLiKfromUA Aug 23, 2024
ff458eb
Merge branch 'p2996' into clang-p2996/issues/5-refined
BaLiKfromUA Aug 26, 2024
7a47147
migrate existing tests to new syntax
BaLiKfromUA Aug 27, 2024
10fff74
test existing functionality related to function pointers
BaLiKfromUA Aug 27, 2024
3c5d8f3
support of pointer to non-static method
BaLiKfromUA Aug 29, 2024
d95299b
test for object with static storage duration holding a pointer to a c…
BaLiKfromUA Aug 29, 2024
4974bfa
address misunderstanding about static pointers + fix bug related to it
BaLiKfromUA Sep 4, 2024
3bab1b1
comments
BaLiKfromUA Sep 4, 2024
562137e
add requested tests related to template functions -- passed
BaLiKfromUA Sep 16, 2024
b4ec538
refactor logic of getting CXXMethodDecl from function pointer
BaLiKfromUA Sep 16, 2024
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
9 changes: 9 additions & 0 deletions clang/include/clang/Basic/DiagnosticMetafnKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ def metafn_returns_non_structural_type : Note<
"represented as a reflection">;
def metafn_invocation_not_constant_expr : Note<
"invocation is not a constant expression">;
def metafn_first_argument_is_not_object : Note<
"expected related object reflection as a first argument for invoking "
"non-static member function">;
def metafn_function_is_not_member_of_object : Note<
"method is not a member of "
"given object reflection">;
def metafn_function_returns_void : Note<
"cannot invoke reflection of void function">;
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved


// Extraction.
def metafn_cannot_extract : Note<
Expand Down
121 changes: 109 additions & 12 deletions clang/lib/Sema/Metafunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4420,6 +4420,25 @@ bool reflect_result(APValue &Result, Sema &S, EvalFn Evaluator,
return SetAndSucceed(Result, APValue(RV));
}

bool is_nonstatic_member_function(ValueDecl *FD) {
if (!FD) {
return false;
}

if (dyn_cast<CXXConstructorDecl>(FD)) {
return false;
}

// todo: check that method is not special??
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved

auto *MD = dyn_cast<CXXMethodDecl>(FD);
if (!MD) {
return false;
}

return !MD->isStatic();
}

bool reflect_invoke(APValue &Result, Sema &S, EvalFn Evaluator,
DiagFn Diagnoser, QualType ResultTy, SourceRange Range,
ArrayRef<Expr *> Args) {
Expand Down Expand Up @@ -4572,10 +4591,17 @@ bool reflect_invoke(APValue &Result, Sema &S, EvalFn Evaluator,
{
sema::TemplateDeductionInfo Info(Args[0]->getExprLoc(),
FTD->getTemplateDepth());

bool exclude_first_arg =
is_nonstatic_member_function(FTD->getTemplatedDecl()) &&
ArgExprs.size() > 0;

TemplateDeductionResult Result = S.DeduceTemplateArguments(
FTD, &ExplicitTAListInfo, ArgExprs, Specialization, Info, false,
true, QualType(), Expr::Classification(),
[](ArrayRef<QualType>) { return false; });
FTD, &ExplicitTAListInfo,
ArrayRef(ArgExprs.begin() + (exclude_first_arg ? 1 : 0),
ArgExprs.end()),
Specialization, Info, false, true, QualType(), Expr::Classification(),
[](ArrayRef<QualType>) { return false; });
if (Result != TemplateDeductionResult::Success)
return Diagnoser(Range.getBegin(), diag::metafn_no_specialization_found)
<< FTD << Range;
Expand All @@ -4595,20 +4621,91 @@ bool reflect_invoke(APValue &Result, Sema &S, EvalFn Evaluator,
ExprResult ER;
{
EnterExpressionEvaluationContext Context(
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
if (auto *DRE = dyn_cast<DeclRefExpr>(FnRefExpr);
DRE && dyn_cast<CXXConstructorDecl>(DRE->getDecl())) {
auto *CtorD = cast<CXXConstructorDecl>(DRE->getDecl());
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);

auto *DRE = dyn_cast<DeclRefExpr>(FnRefExpr);
if (DRE && dyn_cast<CXXConstructorDecl>(DRE->getDecl())) {
auto *CtorD = cast<CXXConstructorDecl>(DRE->getDecl());
ER = S.BuildCXXConstructExpr(
Range.getBegin(), QualType(CtorD->getParent()->getTypeForDecl(), 0),
CtorD, false, ArgExprs, false, false, false, false,
CXXConstructionKind::Complete, Range);
Range.getBegin(), QualType(CtorD->getParent()->getTypeForDecl(), 0),
CtorD, false, ArgExprs, false, false, false, false,
CXXConstructionKind::Complete, Range);
} else {
ER = S.ActOnCallExpr(S.getCurScope(), FnRefExpr, Range.getBegin(),
ArgExprs, Range.getEnd(), /*ExecConfig=*/nullptr);
auto *FnExpr = FnRefExpr;
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved
bool exclude_first_arg = false;

if (DRE && is_nonstatic_member_function(DRE->getDecl())) {
exclude_first_arg = true;
auto *MD = cast<CXXMethodDecl>(DRE->getDecl());

if (ArgExprs.size() < 1) {
// need to have object as a first argument
return Diagnoser(Range.getBegin(),
diag::metafn_first_argument_is_not_object)
<< Range;
}

auto ObjExpr = ArgExprs[0];
auto ObjType = ObjExpr->getType();
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved

if (ObjType->isPointerType()) {
ObjType = ObjType->getPointeeType();
// convert lvalue to rvalue if needed
// since Sema::BuildMemberExpr inside Sema::ActOnMemberAccessExpr
// expects prvalue
ObjExpr = S.DefaultFunctionArrayLvalueConversion(ObjExpr).get();
}

if (!ObjType->getAsCXXRecordDecl()) {
// first argument is not an object
return Diagnoser(Range.getBegin(),
diag::metafn_first_argument_is_not_object)
<< Range;
}

// check that method belongs to class
if (ObjType->getAsCXXRecordDecl() != MD->getParent()) {
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved
return Diagnoser(Range.getBegin(),
diag::metafn_function_is_not_member_of_object)
<< Range;
}

if (MD->getReturnType()->isVoidType()) {
// void return type is not supported
return Diagnoser(Range.getBegin(), diag::metafn_function_returns_void)
<< Range;
}

SourceLocation DummyLoc;
// Hack below is needed to prevent lookup or overload resolution of
// given method reflection. Because this problem has been solved before
// for splice expressions, wrap our decl ref into splice expr and reuse
// specific overload of Sema::ActOnMemberAccessExpr
auto MethodAsSpliceExpr = CXXSpliceExpr::Create(
S.Context, DRE->getValueKind(), DummyLoc, DummyLoc, DRE, DummyLoc,
&ExplicitTAListInfo, /* this arg is not used */ false);

SourceLocation ObjLoc = ObjExpr->getExprLoc();
ExprResult MemberAccessResult = S.ActOnMemberAccessExpr(
S.getCurScope(), ObjExpr, ObjLoc,
ObjExpr->getType()->isPointerType() ? tok::arrow : tok::period,
MethodAsSpliceExpr, DummyLoc);

if (MemberAccessResult.isInvalid()) {
return true;
}

FnExpr = MemberAccessResult.get();
}

ER = S.ActOnCallExpr(
S.getCurScope(), FnExpr, Range.getBegin(),
MutableArrayRef(ArgExprs.begin() + (exclude_first_arg ? 1 : 0),
ArgExprs.end()),
Range.getEnd(), /*ExecConfig=*/nullptr);
}
}

if (ER.isInvalid())
return Diagnoser(Range.getBegin(), diag::metafn_invalid_call_expr) << Range;
Expr *ResultExpr = ER.get();
Expand Down
42 changes: 40 additions & 2 deletions libcxx/test/std/experimental/reflection/reflect-invoke.pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ static_assert([:reflect_invoke(^Cls::fn, {std::meta::reflect_value(4)}):] == 16)
// With reflection of constexpr variable as an argument.
static constexpr int five = 5;
static_assert([:reflect_invoke(^fn1, {^five}):] == 47);

// TODO(P2996): Support nonstatic member functions.
} // namespace basic_functions

// =================
Expand Down Expand Up @@ -241,4 +239,44 @@ static_assert(
std::views::transform(std::meta::reflect_value<int>)));
} // namespace with_non_contiguous_ranges

namespace non_static_member_functions {

class Number {
public:
constexpr Number(int v) : value(v) {}
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved

consteval int plus(int a) const { return plus_impl(a); }

consteval int get_value() const { return value; }

template <typename T>
consteval T multiply(T x) const {
return value * x;
}

private:
consteval int plus_impl(int a) const {
return value + a;
}

const int value;
};

constexpr Number num{42};
constexpr auto num_ref = &num;

static_assert(std::meta::reflect_value(84) ==
reflect_invoke(^Number::plus,
{^num, std::meta::reflect_value(42)}));
static_assert(std::meta::reflect_value(42) ==
reflect_invoke(^Number::get_value, {^num}));

static_assert(std::meta::reflect_value(42) ==
reflect_invoke(^Number::get_value, {^num_ref}));

static_assert(std::meta::reflect_value(84) ==
reflect_invoke(^Number::multiply, {^int},
{^num, std::meta::reflect_value(2)}));
} // namespace non_static_member_functions
BaLiKfromUA marked this conversation as resolved.
Show resolved Hide resolved

int main() { }
65 changes: 65 additions & 0 deletions libcxx/test/std/experimental/reflection/reflect-invoke.verify.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//===----------------------------------------------------------------------===//
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great idea to add these!!

We desperately need more of these "fail-tests" for reflection (I haven't written any) - after you're finishing with this PR, if you have any interest in adding some, that'd be a huge help.

Copy link
Author

@BaLiKfromUA BaLiKfromUA Aug 23, 2024

Choose a reason for hiding this comment

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

I don't mind to work on it!

It would be helpful for me if you could elaborate what testing would be useful for authors of paper and users of this fork.

Should it be only different tests to trigger diagnostic messages or I also need to try to cover some branches where we just say that expression is invalid (return true;)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Both would be great - we don't need full coverage of "expression is invalid" cases for every function, but a few would be nice.

Copy link
Author

Choose a reason for hiding this comment

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

I see, will do after this PR 👍🏻

//
// Copyright 2024 Bloomberg Finance L.P.
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

// UNSUPPORTED: c++03 || c++11 || c++14 || c++17 || c++20
// ADDITIONAL_COMPILE_FLAGS: -freflection
// ADDITIONAL_COMPILE_FLAGS: -Wno-unneeded-internal-declaration -Wno-unused-variable -Wno-unused-value

// <experimental/reflection>
//
// [reflection]

#include <experimental/meta>

namespace NS {
struct A {
consteval int fn() const { return 24; }
};
} // namespace NS

struct A {
consteval int fn() const { return 42; }

consteval void void_fn() const {
// no-op
};
};

struct B {};

int main() {
// ======================
// non-static member functions
// ======================
constexpr A expectedClass{};
reflect_invoke(^A::fn, {^expectedClass}); // ok

reflect_invoke(^A::void_fn, {^expectedClass});
// expected-error-re@-1 {{call to consteval function 'std::meta::reflect_invoke<{{.*}}>' is not a constant expression}}
// expected-note@-2 {{cannot invoke reflection of void function}}

reflect_invoke(^A::fn, {});
// expected-error-re@-1 {{call to consteval function 'std::meta::reflect_invoke<{{.*}}>' is not a constant expression}}
// expected-note@-2 {{expected related object reflection as a first argument for invoking non-static member function}}

reflect_invoke(^A::fn, {std::meta::reflect_value(42)});
// expected-error-re@-1 {{call to consteval function 'std::meta::reflect_invoke<{{.*}}>' is not a constant expression}}
// expected-note@-2 {{expected related object reflection as a first argument for invoking non-static member function}}

constexpr B differentClass{};
reflect_invoke(^A::fn, {^differentClass});
// expected-error-re@-1 {{call to consteval function 'std::meta::reflect_invoke<{{.*}}>' is not a constant expression}}
// expected-note@-2 {{method is not a member of given object reflection}}

constexpr NS::A differentNamespaceClass{};
reflect_invoke(^A::fn, {^differentNamespaceClass});
// expected-error-re@-1 {{call to consteval function 'std::meta::reflect_invoke<{{.*}}>' is not a constant expression}}
// expected-note@-2 {{method is not a member of given object reflection}}
}
Loading