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

[Alexandre Oliva <aoliva at redhat dot com>]

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