| Message ID | ork1u3k95q.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PR,c++/84943] allow folding of array indexing indirect_ref | expand |
On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > fn[0]() ICEs because we end up with addr_expr of a decl, and that > should only happen for artificial or otherwise special internal > functions. For anything else, we should find the decl earlier, but we > don't because we build an indirect_ref or an addr_expr and don't > cancel them out. That's deliberate; we recently changed the C++ front end to defer most folding until genericization time. Jason
On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote: > On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> fn[0]() ICEs because we end up with addr_expr of a decl, and that >> should only happen for artificial or otherwise special internal >> functions. For anything else, we should find the decl earlier, but we >> don't because we build an indirect_ref or an addr_expr and don't >> cancel them out. > That's deliberate; we recently changed the C++ front end to defer most > folding until genericization time. Ok, I can see a number of possibilities as to why this is done, which would lead to different choices in the implemnetation of a fix for this PR: a. to mirror the structure of the program as closely as possible b. to sort-of mirror the structure of the program for the benefit of operator overloading during template expansion c. to allow such constructs as *&x to hide symbolic information about x, so that stuff that isn't part of the type system proper (attributes? concepts?) can be "hidden" by such artifacts Since build_addr_func does discard/fold an INDIRECT_REF and expose the inner ADDR_EXPR and that does the inner FUNCTION_DECL, I'm jumping to the conclusion that the goal was b, and so I put in a loop to cancel out INDIRECT_REF and ADDR_EXPR when they are not template-dependent. If we find a FUNCTION_DECL, we use the info in there, even concepts. If we don't, we retain the function expression we got, except for allowing the simplification of the outermost INDIRECT_REF by build_call_addr. If the goal was a or c, I suppose we should drop the loop altogether, and stop build_addr_call from simplifying a possibly-meaningful INDIRECT_REF. If it was a, we'd probably have to distinguish more clearly between e.g. function-to-pointer decay and explicit unary &, and between dereference and array index. Anyway... Hoping it's b or something else close enough, how's this? Regstrapping on i686- and x86_64-linux-gnu. Ok to install? [PR c++/84943] cancel-out indirect_ref and addr_expr in call fn[0]() ICEs because we end up with addr_expr of a decl, and that should only happen for artificial or otherwise special internal functions. For anything else, we should find the decl earlier, but we don't because we build an indirect_ref of an addr_expr for the array indexing, and we don't want to fold them right away. When building the call, however, we would fold the INDIRECT_REF while building the address for the call, and then we'd find the decl within the remaining ADDR_EXPR, and complain it's a decl but not one of the artificial or special functions. Thus, when building the call, I've introduced code to skip any pairs of cancelling-out INDIRECT_REFs of ADDR_EXPRs, so that we stand a better chance of finding the original fndecl and constructing the call using the fndecl information. If we fail to find the decl, we go back to the original function expression; we'll still simplify the outermost INDIRECT_REF when taking its address, but then we won't find an ADDR_EXPR of a FUNCTION_DECL in there any more. for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_function_call_vec): Cancel-out pairs of INDIRECT_REFs and ADDR_EXPRs to find a function decl. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. --- gcc/cp/typeck.c | 29 ++++++++++++++++++++--------- gcc/testsuite/g++.dg/pr84943.C | 8 ++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d3183b5321d3..f4aa300c4ddb 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3670,7 +3670,19 @@ cp_build_function_call_vec (tree function, vec<tree, va_gc> **params, && TREE_TYPE (function) == TREE_TYPE (TREE_OPERAND (function, 0))) function = TREE_OPERAND (function, 0); - if (TREE_CODE (function) == FUNCTION_DECL) + /* Short-circuit cancelling-out INDIRECT_REFs of ADDR_EXPRs. */ + fndecl = function; + while (TREE_CODE (fndecl) == INDIRECT_REF + && TREE_TYPE (fndecl) + && TREE_CODE (TREE_OPERAND (fndecl, 0)) == ADDR_EXPR + && TREE_TYPE (TREE_OPERAND (fndecl, 0)) + && (TREE_TYPE (fndecl) + == TREE_TYPE (TREE_TYPE (TREE_OPERAND (fndecl, 0)))) + && (TREE_TYPE (fndecl) + == TREE_TYPE (TREE_OPERAND (TREE_OPERAND (fndecl, 0), 0)))) + fndecl = TREE_OPERAND (TREE_OPERAND (fndecl, 0), 0); + + if (TREE_CODE (fndecl) == FUNCTION_DECL) { /* If the function is a non-template member function or a non-template friend, then we need to check the @@ -3683,20 +3695,19 @@ cp_build_function_call_vec (tree function, vec<tree, va_gc> **params, add_function_candidate. */ if (flag_concepts && (complain & tf_error) - && !constraints_satisfied_p (function)) + && !constraints_satisfied_p (fndecl)) { - error ("cannot call function %qD", function); - location_t loc = DECL_SOURCE_LOCATION (function); - diagnose_constraints (loc, function, NULL_TREE); + error ("cannot call function %qD", fndecl); + location_t loc = DECL_SOURCE_LOCATION (fndecl); + diagnose_constraints (loc, fndecl, NULL_TREE); return error_mark_node; } - if (!mark_used (function, complain) && !(complain & tf_error)) + if (!mark_used (fndecl, complain) && !(complain & tf_error)) return error_mark_node; - fndecl = function; /* Convert anything with function type to a pointer-to-function. */ - if (DECL_MAIN_P (function)) + if (DECL_MAIN_P (fndecl)) { if (complain & tf_error) pedwarn (input_location, OPT_Wpedantic, @@ -3704,7 +3715,7 @@ cp_build_function_call_vec (tree function, vec<tree, va_gc> **params, else return error_mark_node; } - function = build_addr_func (function, complain); + function = build_addr_func (fndecl, complain); } else { diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
On Fri, Mar 23, 2018 at 12:17 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> fn[0]() ICEs because we end up with addr_expr of a decl, and that >>> should only happen for artificial or otherwise special internal >>> functions. For anything else, we should find the decl earlier, but we >>> don't because we build an indirect_ref or an addr_expr and don't >>> cancel them out. > >> That's deliberate; we recently changed the C++ front end to defer most >> folding until genericization time. > > Ok, I can see a number of possibilities as to why this is done, which > would lead to different choices in the implemnetation of a fix for this > PR: > > a. to mirror the structure of the program as closely as possible > b. to sort-of mirror the structure of the program for the benefit of > operator overloading during template expansion > > c. to allow such constructs as *&x to hide symbolic information about x, > so that stuff that isn't part of the type system proper (attributes? > concepts?) can be "hidden" by such artifacts Mostly c, as the difference is significant for some language rules. But in some cases, mainly when we're dealing with internally generated trees, we do fold. Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. Jason
On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote: > On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> fn[0]() ICEs because we end up with addr_expr of a decl, and that >> should only happen for artificial or otherwise special internal >> functions. For anything else, we should find the decl earlier, but we >> don't because we build an indirect_ref or an addr_expr and don't >> cancel them out. > That's deliberate; we recently changed the C++ front end to defer most > folding until genericization time. How about this? (not significantly tested yet) [PR c++/84943] keep fndecl hidden from call fn[0]() ICEd because we we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called that way. In order to preserve the program structure and properties that depend on it, we shouldn't cancel out the INDIRECT_REF with the ADDR_EXPR, nor should we bypass them to find the decl. So, make build_addr_func not drop an INDIRECT_REF: when it would do so, wrap its original INDIRECT_REF value in an ADDR_EXPR, then we won't find the decl hiding in there unless the decl was visible to begin with. for gcc/cp/ChangeLog PR c++/84943 * call.c (build_addr_func): If we'd drop an INDIRECT_REF, wrap it in a new ADDR_EXPR instead. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. --- gcc/cp/call.c | 10 +++++++++- gcc/testsuite/g++.dg/pr84943.C | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 9351918b23af..5a0d09b1db4e 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -283,7 +283,15 @@ build_addr_func (tree function, tsubst_flags_t complain) function = build_address (function); } else - function = decay_conversion (function, complain, /*reject_builtin=*/false); + { + tree orig = function; + function = decay_conversion (function, complain, /*reject_builtin=*/false); + + /* Do not cancel out an INDIRECT_REF. */ + if (TREE_CODE (orig) == INDIRECT_REF + && TREE_OPERAND (orig, 0) == function) + function = build1 (ADDR_EXPR, TREE_TYPE (function), orig); + } return function; } diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
On Fri, Mar 23, 2018 at 12:44 PM, Jason Merrill <jason@redhat.com> wrote:
> Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null.
Did you try this? That should avoid it being ADDR_EXPR of a decl.
Jason
On Fri, Mar 23, 2018 at 4:55 PM, Jason Merrill <jason@redhat.com> wrote: > On Fri, Mar 23, 2018 at 12:44 PM, Jason Merrill <jason@redhat.com> wrote: >> Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. > > Did you try this? That should avoid it being ADDR_EXPR of a decl. Oh, I was assuming the ICE was in the middle-end, but it's in build_call_a. And it looks like the problem isn't that it's an ADDR_EXPR of a decl, but that the function isn't marked TREE_USED. Jason
On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote: > On Fri, Mar 23, 2018 at 4:55 PM, Jason Merrill <jason@redhat.com> wrote: >> On Fri, Mar 23, 2018 at 12:44 PM, Jason Merrill <jason@redhat.com> wrote: >>> Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. >> >> Did you try this? That should avoid it being ADDR_EXPR of a decl. > Oh, I was assuming the ICE was in the middle-end, but it's in > build_call_a. And it looks like the problem isn't that it's an > ADDR_EXPR of a decl, but that the function isn't marked TREE_USED. Well, yeah. cp_build_function_call_vec marks the function as used when function is a FUNCTION_DECL. In this testcase, it's INDIRECT_REF of ADDR_EXPR of FUNCTION_DECL. Since the idea of bypassing cancelling-out pairs of INDIRECT_REF and ADDR_EXPR, that would have allowed cp_build_function_call_vec to get to the FUNCTION_DECL and mark it as used was not accepted, the alternative was to stop build_call_a from getting to the FUNCTION_DECL, which was very much in line of what you'd said about preserving source constructs and allowing the significant differences for some language rules to remain in place. Now, to me, it is clear that if we are to preserve source level constructs because they could make some significant different WRT certain language rules, and to that end we don't want to simplify the INDIRECT_REF arising from the array indexing with the ADDR_EXPR of the function-to-pointer decay, then it should follow that we also don't want to simplify the ADDR_EXPR that build_addr_func would introduce with that INDIRECT_REF. That's what the latest patch I proposed does, and it also solves the potential inconsistency between cp_build_function_call_vec and build_call_a, in which one of them does not find the FUNCTION_DECL because it's too deeply hidden within INDIRECT_REFs/ADDR_EXPRs pairs and so it fails to mark the decl as used, but then the other finds it because build_addr_func peeled an INDIRECT_REF, and then complains that the decl is not marked as used. Now, I don't know what the rules are that could make a difference in this case, but I must confess that I'm a bit surprised that the following constructs could possibly be interpreted differently under C++ rules: f(); (&f)(); (*f)(); f[0](); (*&f)(); (*&*&*&f)(); Maybe they aren't when we get to cp_build_function_call_vec (any differences WRT overload resolution would have been taken care of), and we should use get_callee_fndecl in cp_build_function_call_vec, and arrange for get_callee_fndecl to peel as many layers of INDIRECT_REF and ADDR_EXPR as it finds when searching for a FUNCTION_DECL. Anyway, given the accumulated constraints I've been given WRT to this bug, I'm afraid I've run out of ideas. I welcome suggestions as to how to proceed.
On Wed, Mar 28, 2018 at 2:18 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Fri, Mar 23, 2018 at 4:55 PM, Jason Merrill <jason@redhat.com> wrote: >>> On Fri, Mar 23, 2018 at 12:44 PM, Jason Merrill <jason@redhat.com> wrote: >>>> Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. >>> >>> Did you try this? That should avoid it being ADDR_EXPR of a decl. > >> Oh, I was assuming the ICE was in the middle-end, but it's in >> build_call_a. And it looks like the problem isn't that it's an >> ADDR_EXPR of a decl, but that the function isn't marked TREE_USED. > > Well, yeah. cp_build_function_call_vec marks the function as used when > function is a FUNCTION_DECL. In this testcase, it's INDIRECT_REF of > ADDR_EXPR of FUNCTION_DECL. It should have been marked as used before we get to cp_build_function_call_vec. finish_id_expression doesn't mark it because "done" is false, because we're in a postfix-expression. It looks like cp_build_addr_expr_1 already calls mark_used for single static member functions, it should probably do the same for single non-member functions. Jason
On Mar 28, 2018, Jason Merrill <jason@redhat.com> wrote: > It looks like cp_build_addr_expr_1 already calls mark_used for single > static member functions, it should probably do the same for single > non-member functions. Hmm... That existing call is suspicious, now that you point it out. There's a switch before it that peels off BASELINK and takes OVL_FIRST only, so we would be marking as used something other than what overload resolution would select, and not marking what overload resolution would ultimately select, *if* we marked anything. But since we've peeled off the baseline, the test if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) passes because FUNCTION_DECL is not COMPONENT_REF, and we call build_address before we get a chance to mark anything. I guess we could move the mark_used logic ahead in the function, but whether or not it do the job will depend on whether cp_build_addr_expr_1 gets called again after template substitution and overload resolution.
On Mar 29, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 28, 2018, Jason Merrill <jason@redhat.com> wrote: >> It looks like cp_build_addr_expr_1 already calls mark_used for single >> static member functions, it should probably do the same for single >> non-member functions. > ultimately select, *if* we marked anything. But since we've peeled off > the baseline, the test ^ err, baselink > if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) > passes because FUNCTION_DECL is not COMPONENT_REF, and we call > build_address before we get a chance to mark anything. ... unless we're at the case it's supposed to catch, namely when we call obj.static_memfn() ;-) Here's a patch that should take care of the marking a namespace-scoped or static member function as used when taking its address, thus working around (fixing?) the reported problem. Regstrapping now. Ok to install if it passes? [PR c++/84943] mark function as used when taking its address fn[0]() ICEd because we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called without being marked as used. This patch arranges for a FUNCTION_DECL to be marked as used when taking its address, just like we already did when taking the address of a static function to call it as a member function (i.e. using the obj.fn() notation). for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as used. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. * g++.dg/pr84943-2.C: New. --- gcc/cp/typeck.c | 8 ++++- gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/pr84943.C | 8 +++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d3183b5321d3..d6157772c0f3 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -5831,6 +5831,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) if (type_unknown_p (arg)) return build1 (ADDR_EXPR, unknown_type_node, arg); + tree maybe_overloaded_arg = arg; + if (TREE_CODE (arg) == OFFSET_REF) /* We want a pointer to member; bypass all the code for actually taking the address of something. */ @@ -5971,6 +5973,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) so we can just form an ADDR_EXPR with the correct type. */ if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) { + if (TREE_CODE (arg) == FUNCTION_DECL && arg == maybe_overloaded_arg + && !mark_used (arg, complain) && !(complain & tf_error)) + return error_mark_node; val = build_address (arg); if (TREE_CODE (arg) == OFFSET_REF) PTRMEM_OK_P (val) = PTRMEM_OK_P (arg); @@ -5983,7 +5988,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) function. */ gcc_assert (TREE_CODE (fn) == FUNCTION_DECL && DECL_STATIC_FUNCTION_P (fn)); - if (!mark_used (fn, complain) && !(complain & tf_error)) + if (arg == maybe_overloaded_arg + && !mark_used (fn, complain) && !(complain & tf_error)) return error_mark_node; val = build_address (fn); if (TREE_SIDE_EFFECTS (TREE_OPERAND (arg, 0))) diff --git a/gcc/testsuite/g++.dg/pr84943-2.C b/gcc/testsuite/g++.dg/pr84943-2.C new file mode 100644 index 000000000000..d1ef012b9155 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943-2.C @@ -0,0 +1,64 @@ +// { dg-do run } + +// Avoid -pedantic-error default +// { dg-options "" } + +// Make sure the functions referenced by various forms of +// address-taking are marked as used and compiled in. + +static void ac() {} +void a() { + ac[0](); // { dg-warning "arithmetic" } +} + +static void bc() {} +void b() { + (&*&*&*&bc)(); +} + +template <typename U> U cc() {} +void (*c())() { + return cc; +} + +template <typename T> +struct x { + void a(int); + template <typename U> static U a(x*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = x().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(x*) = a; + p3(0); + } +}; + +struct z { + void a(int); + template <typename U> static U a(z*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = z().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(z*) = a; + p3(0); + } +}; + +int main(int argc, char *argv[]) { + if (argc > 1) { + x<void>().a(); + z().a(); + } +} diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
On Mar 29, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > Here's a patch that should take care of the marking a namespace-scoped > or static member function as used when taking its address, thus working > around (fixing?) the reported problem. > Regstrapping now. Ok to install if it passes? It regressed overload/template1.C, because we mark an erroneous template specialization as used when we attempt deduction. The following updated patch avoids that regression, and it has passed bootstrap and regression testing on i686- and x86_64-linux-gnu. Ok to install? [PR c++/84943] mark function as used when taking its address fn[0]() ICEd because we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called without being marked as used. This patch arranges for a FUNCTION_DECL to be marked as used when taking its address, just like we already did when taking the address of a static function to call it as a member function (i.e. using the obj.fn() notation). However, we shouldn't mark functions as used when just performing overload resolution, lest we might instantiate templates we shouldn't, as in g++.dg/overload/template1.C. for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as used. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. * g++.dg/pr84943-2.C: New. --- gcc/cp/typeck.c | 9 +++++ gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/pr84943.C | 8 +++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d3183b5321d3..f6b25c8a837d 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -5831,6 +5831,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) if (type_unknown_p (arg)) return build1 (ADDR_EXPR, unknown_type_node, arg); + tree maybe_overloaded_arg = arg; + if (TREE_CODE (arg) == OFFSET_REF) /* We want a pointer to member; bypass all the code for actually taking the address of something. */ @@ -5971,6 +5973,10 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) so we can just form an ADDR_EXPR with the correct type. */ if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) { + if (TREE_CODE (arg) == FUNCTION_DECL + && arg == maybe_overloaded_arg && !(complain & tf_conv) + && !mark_used (arg, complain) && !(complain & tf_error)) + return error_mark_node; val = build_address (arg); if (TREE_CODE (arg) == OFFSET_REF) PTRMEM_OK_P (val) = PTRMEM_OK_P (arg); @@ -5983,7 +5989,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) function. */ gcc_assert (TREE_CODE (fn) == FUNCTION_DECL && DECL_STATIC_FUNCTION_P (fn)); - if (!mark_used (fn, complain) && !(complain & tf_error)) + if (arg == maybe_overloaded_arg && !(complain & tf_conv) + && !mark_used (fn, complain) && !(complain & tf_error)) return error_mark_node; val = build_address (fn); if (TREE_SIDE_EFFECTS (TREE_OPERAND (arg, 0))) diff --git a/gcc/testsuite/g++.dg/pr84943-2.C b/gcc/testsuite/g++.dg/pr84943-2.C new file mode 100644 index 000000000000..d1ef012b9155 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943-2.C @@ -0,0 +1,64 @@ +// { dg-do run } + +// Avoid -pedantic-error default +// { dg-options "" } + +// Make sure the functions referenced by various forms of +// address-taking are marked as used and compiled in. + +static void ac() {} +void a() { + ac[0](); // { dg-warning "arithmetic" } +} + +static void bc() {} +void b() { + (&*&*&*&bc)(); +} + +template <typename U> U cc() {} +void (*c())() { + return cc; +} + +template <typename T> +struct x { + void a(int); + template <typename U> static U a(x*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = x().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(x*) = a; + p3(0); + } +}; + +struct z { + void a(int); + template <typename U> static U a(z*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = z().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(z*) = a; + p3(0); + } +}; + +int main(int argc, char *argv[]) { + if (argc > 1) { + x<void>().a(); + z().a(); + } +} diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
On Fri, Mar 30, 2018 at 3:48 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 29, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > >> Here's a patch that should take care of the marking a namespace-scoped >> or static member function as used when taking its address, thus working >> around (fixing?) the reported problem. > >> Regstrapping now. Ok to install if it passes? > > It regressed overload/template1.C, because we mark an erroneous template > specialization as used when we attempt deduction. > > The following updated patch avoids that regression, and it has passed > bootstrap and regression testing on i686- and x86_64-linux-gnu. Ok to install? > > [PR c++/84943] mark function as used when taking its address > > fn[0]() ICEd because we would fold the INDIRECT_REF used for the > array indexing while building the address for the call, after not > finding the decl hiding there at first. But the decl would be exposed > by the folding, and then lower layers would complain we had the decl, > after all, but it wasn't one of the artificial or special functions > that could be called without being marked as used. > > This patch arranges for a FUNCTION_DECL to be marked as used when > taking its address, just like we already did when taking the address > of a static function to call it as a member function (i.e. using the > obj.fn() notation). However, we shouldn't mark functions as used when > just performing overload resolution, lest we might instantiate > templates we shouldn't, as in g++.dg/overload/template1.C. > > > for gcc/cp/ChangeLog > > PR c++/84943 > * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as > used. > > for gcc/testsuite/ChangeLog > > PR c++/84943 > * g++.dg/pr84943.C: New. > * g++.dg/pr84943-2.C: New. > --- > gcc/cp/typeck.c | 9 +++++ > gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ > gcc/testsuite/g++.dg/pr84943.C | 8 +++++ > 3 files changed, 80 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C > create mode 100644 gcc/testsuite/g++.dg/pr84943.C > > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c > index d3183b5321d3..f6b25c8a837d 100644 > --- a/gcc/cp/typeck.c > +++ b/gcc/cp/typeck.c > @@ -5831,6 +5831,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) > if (type_unknown_p (arg)) > return build1 (ADDR_EXPR, unknown_type_node, arg); > > + tree maybe_overloaded_arg = arg; I don't think we need this; if arg is overloaded, we take the type_unknown_p early exit, so the code lower down is always dealing with a single function. Jason
On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote: > I don't think we need this; if arg is overloaded, we take the > type_unknown_p early exit, so the code lower down is always dealing > with a single function. Aah, that's why it seemed to me that we had already resolved overloads when we got there. As a bonus, I added the tf_conv test before another mark_used call I'd missed in the previous patch. Regstrapped on i686- and x86_64-linux-gnu. Ok to install? [PR c++/84943] mark function as used when taking its address fn[0]() ICEd because we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called without being marked as used. This patch arranges for a FUNCTION_DECL to be marked as used when taking its address, just like we already did when taking the address of a static function to call it as a member function (i.e. using the obj.fn() notation). However, we shouldn't mark functions as used when just performing overload resolution, lest we might instantiate templates we shouldn't, as in g++.dg/overload/template1.C. for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as used. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. * g++.dg/pr84943-2.C: New. --- gcc/cp/typeck.c | 9 ++++- gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/pr84943.C | 8 +++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d454c6c5a295..bdb2bb30a583 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -5801,7 +5801,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) and the created OFFSET_REF. */ tree base = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg, 0))); tree fn = get_first_fn (TREE_OPERAND (arg, 1)); - if (!mark_used (fn, complain) && !(complain & tf_error)) + if (!(complain & tf_conv) + && !mark_used (fn, complain) && !(complain & tf_error)) return error_mark_node; if (! flag_ms_extensions) @@ -5971,6 +5972,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) so we can just form an ADDR_EXPR with the correct type. */ if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) { + if (TREE_CODE (arg) == FUNCTION_DECL && !(complain & tf_conv) + && !mark_used (arg, complain) && !(complain & tf_error)) + return error_mark_node; val = build_address (arg); if (TREE_CODE (arg) == OFFSET_REF) PTRMEM_OK_P (val) = PTRMEM_OK_P (arg); @@ -5983,7 +5987,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) function. */ gcc_assert (TREE_CODE (fn) == FUNCTION_DECL && DECL_STATIC_FUNCTION_P (fn)); - if (!mark_used (fn, complain) && !(complain & tf_error)) + if (!(complain & tf_conv) + && !mark_used (fn, complain) && !(complain & tf_error)) return error_mark_node; val = build_address (fn); if (TREE_SIDE_EFFECTS (TREE_OPERAND (arg, 0))) diff --git a/gcc/testsuite/g++.dg/pr84943-2.C b/gcc/testsuite/g++.dg/pr84943-2.C new file mode 100644 index 000000000000..d1ef012b9155 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943-2.C @@ -0,0 +1,64 @@ +// { dg-do run } + +// Avoid -pedantic-error default +// { dg-options "" } + +// Make sure the functions referenced by various forms of +// address-taking are marked as used and compiled in. + +static void ac() {} +void a() { + ac[0](); // { dg-warning "arithmetic" } +} + +static void bc() {} +void b() { + (&*&*&*&bc)(); +} + +template <typename U> U cc() {} +void (*c())() { + return cc; +} + +template <typename T> +struct x { + void a(int); + template <typename U> static U a(x*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = x().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(x*) = a; + p3(0); + } +}; + +struct z { + void a(int); + template <typename U> static U a(z*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = z().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(z*) = a; + p3(0); + } +}; + +int main(int argc, char *argv[]) { + if (argc > 1) { + x<void>().a(); + z().a(); + } +} diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
On Sat, Mar 31, 2018 at 2:24 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote: > >> I don't think we need this; if arg is overloaded, we take the >> type_unknown_p early exit, so the code lower down is always dealing >> with a single function. > > Aah, that's why it seemed to me that we had already resolved overloads > when we got there. > > As a bonus, I added the tf_conv test before another mark_used call I'd > missed in the previous patch. What if we check tf_conv in mark_used itself rather than at all the call sites? Jason
On Apr 2, 2018, Jason Merrill <jason@redhat.com> wrote: > On Sat, Mar 31, 2018 at 2:24 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote: >> >>> I don't think we need this; if arg is overloaded, we take the >>> type_unknown_p early exit, so the code lower down is always dealing >>> with a single function. >> >> Aah, that's why it seemed to me that we had already resolved overloads >> when we got there. >> >> As a bonus, I added the tf_conv test before another mark_used call I'd >> missed in the previous patch. > What if we check tf_conv in mark_used itself rather than at all the call sites? There are other uses of mark_used, but presumably we want them all to be more conservative under tf_conv, so... Here's what I'm testing. Ok if it passes? [PR c++/84943] mark function as used when taking its address fn[0]() ICEd because we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called without being marked as used. This patch arranges for a FUNCTION_DECL to be marked as used when taking its address, just like we already did when taking the address of a static function to call it as a member function (i.e. using the obj.fn() notation). However, we shouldn't mark functions as used when just performing overload resolution, lest we might instantiate templates we shouldn't, as in g++.dg/overload/template1.C, so we adjust mark_used to return early when testing conversions. for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as used. * decl2.c (mark_used): Return without effects if tf_conv. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. * g++.dg/pr84943-2.C: New. --- gcc/cp/decl2.c | 6 ++++ gcc/cp/typeck.c | 3 ++ gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/pr84943.C | 8 +++++ 4 files changed, 81 insertions(+) create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index fa753749e1a6..740e85b35617 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -5201,6 +5201,12 @@ maybe_instantiate_decl (tree decl) bool mark_used (tree decl, tsubst_flags_t complain) { + /* If we're just testing conversions or resolving overloads, we + don't want any permanent effects like forcing functions to be + output or instantiating templates. */ + if ((complain & tf_conv)) + return false; + /* If DECL is a BASELINK for a single function, then treat it just like the DECL for the function. Otherwise, if the BASELINK is for an overloaded function, we don't know which function was diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d454c6c5a295..bd67b82fcc65 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -5971,6 +5971,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) so we can just form an ADDR_EXPR with the correct type. */ if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) { + if (TREE_CODE (arg) == FUNCTION_DECL + && !mark_used (arg, complain) && !(complain & tf_error)) + return error_mark_node; val = build_address (arg); if (TREE_CODE (arg) == OFFSET_REF) PTRMEM_OK_P (val) = PTRMEM_OK_P (arg); diff --git a/gcc/testsuite/g++.dg/pr84943-2.C b/gcc/testsuite/g++.dg/pr84943-2.C new file mode 100644 index 000000000000..d1ef012b9155 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943-2.C @@ -0,0 +1,64 @@ +// { dg-do run } + +// Avoid -pedantic-error default +// { dg-options "" } + +// Make sure the functions referenced by various forms of +// address-taking are marked as used and compiled in. + +static void ac() {} +void a() { + ac[0](); // { dg-warning "arithmetic" } +} + +static void bc() {} +void b() { + (&*&*&*&bc)(); +} + +template <typename U> U cc() {} +void (*c())() { + return cc; +} + +template <typename T> +struct x { + void a(int); + template <typename U> static U a(x*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = x().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(x*) = a; + p3(0); + } +}; + +struct z { + void a(int); + template <typename U> static U a(z*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = z().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(z*) = a; + p3(0); + } +}; + +int main(int argc, char *argv[]) { + if (argc > 1) { + x<void>().a(); + z().a(); + } +} diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
On Tue, Apr 3, 2018 at 3:44 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Apr 2, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On Sat, Mar 31, 2018 at 2:24 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote: >>> >>>> I don't think we need this; if arg is overloaded, we take the >>>> type_unknown_p early exit, so the code lower down is always dealing >>>> with a single function. >>> >>> Aah, that's why it seemed to me that we had already resolved overloads >>> when we got there. >>> >>> As a bonus, I added the tf_conv test before another mark_used call I'd >>> missed in the previous patch. > >> What if we check tf_conv in mark_used itself rather than at all the call sites? > > There are other uses of mark_used, but presumably we want them all to > be more conservative under tf_conv, so... Here's what I'm testing. Ok > if it passes? > > > [PR c++/84943] mark function as used when taking its address > > fn[0]() ICEd because we would fold the INDIRECT_REF used for the > array indexing while building the address for the call, after not > finding the decl hiding there at first. But the decl would be exposed > by the folding, and then lower layers would complain we had the decl, > after all, but it wasn't one of the artificial or special functions > that could be called without being marked as used. > > This patch arranges for a FUNCTION_DECL to be marked as used when > taking its address, just like we already did when taking the address > of a static function to call it as a member function (i.e. using the > obj.fn() notation). However, we shouldn't mark functions as used when > just performing overload resolution, lest we might instantiate > templates we shouldn't, as in g++.dg/overload/template1.C, so we > adjust mark_used to return early when testing conversions. > > > for gcc/cp/ChangeLog > > PR c++/84943 > * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as > used. > * decl2.c (mark_used): Return without effects if tf_conv. > > for gcc/testsuite/ChangeLog > > PR c++/84943 > * g++.dg/pr84943.C: New. > * g++.dg/pr84943-2.C: New. > --- > gcc/cp/decl2.c | 6 ++++ > gcc/cp/typeck.c | 3 ++ > gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ > gcc/testsuite/g++.dg/pr84943.C | 8 +++++ > 4 files changed, 81 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C > create mode 100644 gcc/testsuite/g++.dg/pr84943.C > > diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c > index fa753749e1a6..740e85b35617 100644 > --- a/gcc/cp/decl2.c > +++ b/gcc/cp/decl2.c > @@ -5201,6 +5201,12 @@ maybe_instantiate_decl (tree decl) > bool > mark_used (tree decl, tsubst_flags_t complain) > { > + /* If we're just testing conversions or resolving overloads, we > + don't want any permanent effects like forcing functions to be > + output or instantiating templates. */ > + if ((complain & tf_conv)) > + return false; I think we want to return true. Jason
On Tue, Apr 3, 2018 at 11:47 AM, Jason Merrill <jason@redhat.com> wrote: > On Tue, Apr 3, 2018 at 3:44 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Apr 2, 2018, Jason Merrill <jason@redhat.com> wrote: >> >>> On Sat, Mar 31, 2018 at 2:24 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>>> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote: >>>> >>>>> I don't think we need this; if arg is overloaded, we take the >>>>> type_unknown_p early exit, so the code lower down is always dealing >>>>> with a single function. >>>> >>>> Aah, that's why it seemed to me that we had already resolved overloads >>>> when we got there. >>>> >>>> As a bonus, I added the tf_conv test before another mark_used call I'd >>>> missed in the previous patch. >> >>> What if we check tf_conv in mark_used itself rather than at all the call sites? >> >> There are other uses of mark_used, but presumably we want them all to >> be more conservative under tf_conv, so... Here's what I'm testing. Ok >> if it passes? >> >> >> [PR c++/84943] mark function as used when taking its address >> >> fn[0]() ICEd because we would fold the INDIRECT_REF used for the >> array indexing while building the address for the call, after not >> finding the decl hiding there at first. But the decl would be exposed >> by the folding, and then lower layers would complain we had the decl, >> after all, but it wasn't one of the artificial or special functions >> that could be called without being marked as used. >> >> This patch arranges for a FUNCTION_DECL to be marked as used when >> taking its address, just like we already did when taking the address >> of a static function to call it as a member function (i.e. using the >> obj.fn() notation). However, we shouldn't mark functions as used when >> just performing overload resolution, lest we might instantiate >> templates we shouldn't, as in g++.dg/overload/template1.C, so we >> adjust mark_used to return early when testing conversions. >> >> >> for gcc/cp/ChangeLog >> >> PR c++/84943 >> * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as >> used. >> * decl2.c (mark_used): Return without effects if tf_conv. >> >> for gcc/testsuite/ChangeLog >> >> PR c++/84943 >> * g++.dg/pr84943.C: New. >> * g++.dg/pr84943-2.C: New. >> --- >> gcc/cp/decl2.c | 6 ++++ >> gcc/cp/typeck.c | 3 ++ >> gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ >> gcc/testsuite/g++.dg/pr84943.C | 8 +++++ >> 4 files changed, 81 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C >> create mode 100644 gcc/testsuite/g++.dg/pr84943.C >> >> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c >> index fa753749e1a6..740e85b35617 100644 >> --- a/gcc/cp/decl2.c >> +++ b/gcc/cp/decl2.c >> @@ -5201,6 +5201,12 @@ maybe_instantiate_decl (tree decl) >> bool >> mark_used (tree decl, tsubst_flags_t complain) >> { >> + /* If we're just testing conversions or resolving overloads, we >> + don't want any permanent effects like forcing functions to be >> + output or instantiating templates. */ >> + if ((complain & tf_conv)) >> + return false; > > I think we want to return true. (OK with that change.) Jason
On Apr 3, 2018, Jason Merrill <jason@redhat.com> wrote: > On Tue, Apr 3, 2018 at 11:47 AM, Jason Merrill <jason@redhat.com> wrote: >> On Tue, Apr 3, 2018 at 3:44 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> + if ((complain & tf_conv)) >>> + return false; >> I think we want to return true. Yeah, it doesn't work at all returning false. > (OK with that change.) Thanks, here's what I'm installing. [PR c++/84943] mark function as used when taking its address fn[0]() ICEd because we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called without being marked as used. This patch arranges for a FUNCTION_DECL to be marked as used when taking its address, just like we already did when taking the address of a static function to call it as a member function (i.e. using the obj.fn() notation). However, we shouldn't mark functions as used when just performing overload resolution, lest we might instantiate templates we shouldn't, as in g++.dg/overload/template1.C, so we adjust mark_used to return early when testing conversions. for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_addr_expr_1): Mark FUNCTION_DECL as used. * decl2.c (mark_used): Return without effects if tf_conv. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. * g++.dg/pr84943-2.C: New. --- gcc/cp/decl2.c | 6 ++++ gcc/cp/typeck.c | 3 ++ gcc/testsuite/g++.dg/pr84943-2.C | 64 ++++++++++++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/pr84943.C | 8 +++++ 4 files changed, 81 insertions(+) create mode 100644 gcc/testsuite/g++.dg/pr84943-2.C create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index fa753749e1a6..6ae6cef78dda 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -5201,6 +5201,12 @@ maybe_instantiate_decl (tree decl) bool mark_used (tree decl, tsubst_flags_t complain) { + /* If we're just testing conversions or resolving overloads, we + don't want any permanent effects like forcing functions to be + output or instantiating templates. */ + if ((complain & tf_conv)) + return true; + /* If DECL is a BASELINK for a single function, then treat it just like the DECL for the function. Otherwise, if the BASELINK is for an overloaded function, we don't know which function was diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d454c6c5a295..bd67b82fcc65 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -5971,6 +5971,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain) so we can just form an ADDR_EXPR with the correct type. */ if (processing_template_decl || TREE_CODE (arg) != COMPONENT_REF) { + if (TREE_CODE (arg) == FUNCTION_DECL + && !mark_used (arg, complain) && !(complain & tf_error)) + return error_mark_node; val = build_address (arg); if (TREE_CODE (arg) == OFFSET_REF) PTRMEM_OK_P (val) = PTRMEM_OK_P (arg); diff --git a/gcc/testsuite/g++.dg/pr84943-2.C b/gcc/testsuite/g++.dg/pr84943-2.C new file mode 100644 index 000000000000..d1ef012b9155 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943-2.C @@ -0,0 +1,64 @@ +// { dg-do run } + +// Avoid -pedantic-error default +// { dg-options "" } + +// Make sure the functions referenced by various forms of +// address-taking are marked as used and compiled in. + +static void ac() {} +void a() { + ac[0](); // { dg-warning "arithmetic" } +} + +static void bc() {} +void b() { + (&*&*&*&bc)(); +} + +template <typename U> U cc() {} +void (*c())() { + return cc; +} + +template <typename T> +struct x { + void a(int); + template <typename U> static U a(x*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = x().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(x*) = a; + p3(0); + } +}; + +struct z { + void a(int); + template <typename U> static U a(z*) {} + static void a(long) {} + static void a(void *) {} + static void a() { + void (*p0)(void*) = z().a; + p0(0); + void (*p1)(long) = a; + p1(0); + void (*p2)() = a; + p2(); + void (*p3)(z*) = a; + p3(0); + } +}; + +int main(int argc, char *argv[]) { + if (argc > 1) { + x<void>().a(); + z().a(); + } +} diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d3183b5321d3..5d08cb78a388 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3393,11 +3393,11 @@ cp_build_array_ref (location_t loc, tree array, tree idx, warn_array_subscript_with_type_char (loc, idx); - ret = cp_build_indirect_ref (cp_build_binary_op (input_location, - PLUS_EXPR, ar, ind, - complain), - RO_ARRAY_INDEXING, - complain); + ret = cp_build_indirect_ref_1 (cp_build_binary_op (input_location, + PLUS_EXPR, ar, ind, + complain), + RO_ARRAY_INDEXING, + complain, true); protected_set_expr_location (ret, loc); if (non_lvalue) ret = non_lvalue_loc (loc, ret); diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index 000000000000..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +}