On Tue, Jun 9, 2020 at 5:04 AM Iain Sandoe <i...@sandoe.co.uk> wrote:
> Hi C++ folks, > > There are a number of places in the coroutine codegen where we need to > build calls to promise methods. > > Such methods can, in several cases, also be constexpr or static (or both) > which leads to the PR, > > Now the mechanism I use is this; > > 1/ lookup the method in the relevant scope > > tree method > = lookup_member (promise, member_id, /*protect=*/1, /*want_type=*/0, > tf_warning_or_error); > > - this returns a BASELINK tree.. > > 2/ In many cases, the calls are to methods with no parms. > > so, with a dummy object, I do: > > tree call > = build_new_method_call (dummy, method, NULL, NULL_TREE, LOOKUP_NORMAL, > NULL, flags); > > ‘args’ is the 3rd parm. > > 3/ The header comment on build_new_method_call says: > > /* Build a call to "INSTANCE.FN (ARGS)". If FN_P is non-NULL, it will > be set, upon return, to the function called. ARGS may be NULL. > This may change ARGS. */ > > Which says that the 3rd arg can validly be NULL (as per my example). > > 4/ however, inside build_new_method_call ‘args' gets assigned to > ‘user_args’ which is used in a lot of places without any guard to see if > it’s non-null. One of those places is passing it to add_candidates (), > which eventually in the case of a static method uses the pointer and ICEs. > > ===== > > So… > > A/ I’m doing something wrong, and my use of APIs is violating an > expectation (in which case what’s the right API usage?) > > B/ It happens to be other callers to build_new_method_call() have either > had dummy args or not produced the same set of conditions > It does seem that most callers use dummy args, but the function isn't documented that way, and it seems preferable to allow null args. (for my case of static promise methods) > > * if I provide an empty dummy ‘args' to the build_new_method_call it > works > * if I fix the unguarded use of ‘args' in add_candidates (as below) it > works, > > AFAICT, the intent is to avoid every caller for build_new_method_call() > having to build a dummy argument list when that list is empty - it would > seem irritating to have to add one for every use in coros - but I can. > > WDYT? > thanks > Iain > > -------- > > the fix for call.c: > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 3c97b9846e2..dccc1bf596f 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -5862,7 +5862,7 @@ add_candidates (tree fns, tree first_arg, const > vec<tree, va_gc> *args, > } > > /* Don't bother reversing an operator with two identical > parameters. */ > - else if (args->length () == 2 && (flags & LOOKUP_REVERSED)) > + else if (args && args->length () == 2 && (flags & LOOKUP_REVERSED)) > The usual pattern is to use vec_safe_length here. Similarly, in build_new_method_call_1 I think !user_args->is_empty() should be vec_safe_is_empty (user_args) Those changes are OK. Jason
