[PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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.  Let fold do its job when building the array indexing
indirect_ref, and the ICE is gone.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?

for  gcc/cp/ChangeLog

	PR c++/84943
	* typeck.c (cp_build_array_ref): Allow the indirect_ref to be
	folded.

for  gcc/testsuite/ChangeLog

	PR c++/84943
	* g++.dg/pr84943.C: New.
---
 gcc/cp/typeck.c                |   10 +++++-----
 gcc/testsuite/g++.dg/pr84943.C |    8 ++++++++
 2 files changed, 13 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr84943.C

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Jason Merrill]

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

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref

[Alexandre Oliva]

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" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer