On 10/20/23 17:37, Patrick Palka wrote:
On Fri, 20 Oct 2023, Patrick Palka wrote:
On Fri, 20 Oct 2023, Patrick Palka wrote:
On Fri, 20 Oct 2023, Ken Matsui wrote:
This patch implements built-in trait for std::is_invocable.
Nice! My email client unfortunately ate my first review attempt, so
apologies for my brevity this time around.
gcc/cp/ChangeLog:
* cp-trait.def: Define __is_invocable.
* constraint.cc (diagnose_trait_expr): Handle CPTK_IS_INVOCABLE.
* semantics.cc (trait_expr_value): Likewise.
(finish_trait_expr): Likewise.
(is_invocable_p): New function.
* method.h: New file to export build_trait_object in method.cc.
Given how much larger semantics.cc is than method.cc, maybe let's put
is_invocable_p in method.cc instead? And in general declarations can go
in cp-tree.h.
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 7cccbae5287..cc2e400531a 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -45,6 +45,10 @@ along with GCC; see the file COPYING3. If not see
#include "gomp-constants.h"
#include "predict.h"
#include "memmodel.h"
+#include "method.h"
+
+#include "print-tree.h"
+#include "tree-pretty-print.h"
/* There routines provide a modular interface to perform many parsing
operations. They may therefore be used during actual parsing, or
@@ -11714,6 +11718,133 @@ classtype_has_nothrow_assign_or_copy_p (tree type,
bool assign_p)
return saw_copy;
}
+/* Return true if FN_TYPE is invocable with the given ARG_TYPES. */
+
+static bool
+is_invocable_p (tree fn_type, tree arg_types)
(Sorry for the spam) We'll eventually want to implement a built-in for
invoke_result, so perhaps we should preemptively factor out the bulk
of this function into a 'build_INVOKE' helper function that returns the
built tree?
+{
+ /* ARG_TYPES must be a TREE_VEC. */
+ gcc_assert (TREE_CODE (arg_types) == TREE_VEC);
+
+ /* Access check is required to determine if the given is invocable. */
+ deferring_access_check_sentinel acs (dk_no_deferred);
+
+ /* std::is_invocable is an unevaluated context. */
+ cp_unevaluated cp_uneval_guard;
+
+ bool is_ptrdatamem;
+ bool is_ptrmemfunc;
+ if (TREE_CODE (fn_type) == REFERENCE_TYPE)
+ {
+ tree deref_fn_type = TREE_TYPE (fn_type);
+ is_ptrdatamem = TYPE_PTRDATAMEM_P (deref_fn_type);
+ is_ptrmemfunc = TYPE_PTRMEMFUNC_P (deref_fn_type);
+
+ /* Dereference fn_type if it is a pointer to member. */
+ if (is_ptrdatamem || is_ptrmemfunc)
+ fn_type = deref_fn_type;
+ }
+ else
+ {
+ is_ptrdatamem = TYPE_PTRDATAMEM_P (fn_type);
+ is_ptrmemfunc = TYPE_PTRMEMFUNC_P (fn_type);
+ }
+
+ if (is_ptrdatamem && TREE_VEC_LENGTH (arg_types) != 1)
+ /* A pointer to data member with non-one argument is not invocable. */
+ return false;
+
+ if (is_ptrmemfunc && TREE_VEC_LENGTH (arg_types) == 0)
+ /* A pointer to member function with no arguments is not invocable. */
+ return false;
+
+ /* Construct an expression of a pointer to member. */
+ tree datum;
+ if (is_ptrdatamem || is_ptrmemfunc)
+ {
+ tree datum_type = TREE_VEC_ELT (arg_types, 0);
+
+ /* Dereference datum. */
+ if (CLASS_TYPE_P (datum_type))
+ {
+ bool is_refwrap = false;
+
+ tree datum_decl = TYPE_NAME (TYPE_MAIN_VARIANT (datum_type));
+ if (decl_in_std_namespace_p (datum_decl))
+ {
+ tree name = DECL_NAME (datum_decl);
+ if (name && (id_equal (name, "reference_wrapper")))
+ {
+ /* Handle std::reference_wrapper. */
+ is_refwrap = true;
+ datum_type = cp_build_reference_type (datum_type, false);
Why do you change datum_type from std::reference_wrapper<...> to
std::reference_wrapper<...>&?
+ }
+ }
+
+ datum = build_trait_object (datum_type);
+
+ /* If datum_type was not std::reference_wrapper, check if it has
+ operator*() overload. If datum_type was std::reference_wrapper,
+ avoid dereferencing the datum twice. */
+ if (!is_refwrap)
+ if (get_class_binding (datum_type, get_identifier ("operator*")))
We probably should use lookup_member instead of get_class_binding since
IIUC the latter doesn't look into bases:
struct A { int m; };
struct B { A& operator*(): };
struct C : B { };
static_assert(std::is_invocable_v<int A::*, C>);
However, I notice that the specification of INVOKE
(https://eel.is/c++draft/func.require#lib:INVOKE) doesn't mention name
lookup at all so it strikes me as suspicious that we'd perform name
lookup here.
Agreed. It seems that whether or not to build_x_indirect_ref should
depend instead on whether f is a pointer to a member of decltype(t1) (as
well as is_refwrap).
I think this would misbehave for:
struct A { };
struct B : A { A& operator*() = delete; };
static_assert(std::is_invocable_v<int A::*, B>);
struct C : private A { A& operator*(); };
static_assert(std::is_invocable_v<int A::*, C>);
Oops, this static_assert is missing a !
ultimately because we end up choosing the dereference form of INVOKE,
but according to 1.1/1.4 we should choose the non-dereference form?
+ /* Handle operator*(). */
+ datum = build_x_indirect_ref (UNKNOWN_LOCATION, datum,
+ RO_UNARY_STAR, NULL_TREE,
+ tf_none);
+ }
+ else if (POINTER_TYPE_P (datum_type))
+ datum = build_trait_object (TREE_TYPE (datum_type));
+ else
+ datum = build_trait_object (datum_type);
+ }
+
+ /* Build a function expression. */
+ tree fn;
+ if (is_ptrdatamem)
+ fn = build_m_component_ref (datum, build_trait_object (fn_type), tf_none);
Maybe exit early for the is_ptrdatamem case here (and simplify the rest
of the function accordingly)?
+ else if (is_ptrmemfunc)
+ fn = build_trait_object (TYPE_PTRMEMFUNC_FN_TYPE (fn_type));
Why not use build_m_component_ref and build_offset_ref_call_from_tree
like it would if you wrote (t1.*f)() directly?
Jason
