Re: [PATCH 1/2] c++: remove NON_DEPENDENT_EXPR, part 1

2023-09-27 Thread Patrick Palka
On Tue, 26 Sep 2023, Jason Merrill wrote:

> On 9/25/23 16:43, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > for trunk?
> > 
> > -- >8 --
> > 
> > This tree code dates all the way back to r69130[1] which implemented
> > typing of non-dependent expressions.  Its motivation was never clear (to
> > me at least) since the documentation for it in e.g. cp-tree.def doesn't
> > seem accurate anymore.  build_non_dependent_expr has since gained
> > a bunch of edge cases about whether (or how) to wrap certain templated
> > trees, making it hard to reason about in general.
> > 
> > So this patch removes this tree code, and temporarily turns
> > build_non_dependent_expr into the identity function.  The subsequent
> > patch will remove build_non_dependent_expr and adjust its callers
> > appropriately.
> > 
> > We now need to gracefully handle templated (sub)trees in a couple of
> > places, places which previously didn't see templated trees since they
> > didn't look through NON_DEPENDENT_EXPR.
> > 
> > [1]: https://gcc.gnu.org/pipermail/gcc-patches/2003-July/109355.html
> > 
> > gcc/c-family/ChangeLog:
> > 
> > * c-warn.cc (check_address_or_pointer_of_packed_member): Handle
> > templated CALL_EXPR naming a local extern function.
> > 
> > gcc/cp/ChangeLog:
> > 
> > * class.cc (instantiate_type): Remove NON_DEPENDENT_EXPR
> > handling.
> > * constexpr.cc (cxx_eval_constant_expression): Likewise.
> > (potential_constant_expression_1): Likewise.
> > * coroutines.cc (coro_validate_builtin_call): Don't
> > expect ALIGNOF_EXPR to be wrapped in NON_DEPENDENT_EXPR.
> > * cp-objcp-common.cc (cp_common_init_ts): Remove
> > NON_DEPENDENT_EXPR handling.
> > * cp-tree.def (NON_DEPENDENT_EXPR): Remove.
> > * cp-tree.h (build_non_dependent_expr): Temporarily redefine as
> > the identity function.
> > * cvt.cc (maybe_warn_nodiscard): Handle templated CALL_EXPR
> > naming a local extern function.
> > * cxx-pretty-print.cc (cxx_pretty_printer::expression): Remove
> > NON_DEPENDENT_EXPR handling.
> > * error.cc (dump_decl): Likewise.
> > (dump_expr): Likewise.
> > * expr.cc (mark_use): Likewise.
> > (mark_exp_read): Likewise.
> > * pt.cc (build_non_dependent_expr): Remove.
> > * tree.cc (lvalue_kind): Remove NON_DEPENDENT_EXPR handling.
> > (cp_stabilize_reference): Likewise.
> > * typeck.cc (warn_for_null_address): Likewise.
> > (cp_build_binary_op): Handle type-dependent SIZEOF_EXPR operands.
> > (cp_build_unary_op) : Don't fold inside a
> > template.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/concepts/var-concept3.C: Adjust expected diagnostic
> > for attempting to call a variable concept.
> > ---
> >   gcc/c-family/c-warn.cc   |  2 +-
> >   gcc/cp/class.cc  |  9 --
> >   gcc/cp/constexpr.cc  |  9 --
> >   gcc/cp/coroutines.cc |  3 +-
> >   gcc/cp/cp-objcp-common.cc|  1 -
> >   gcc/cp/cp-tree.def   | 11 ---
> >   gcc/cp/cp-tree.h |  2 +-
> >   gcc/cp/cvt.cc|  4 +-
> >   gcc/cp/cxx-pretty-print.cc   |  1 -
> >   gcc/cp/error.cc  |  8 --
> >   gcc/cp/expr.cc   |  2 -
> >   gcc/cp/pt.cc | 92 
> >   gcc/cp/tree.cc   |  5 --
> >   gcc/cp/typeck.cc | 13 +--
> >   gcc/testsuite/g++.dg/concepts/var-concept3.C |  2 +-
> >   15 files changed, 15 insertions(+), 149 deletions(-)
> > 
> > diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
> > index e67dd87a773..c07770394bf 100644
> > --- a/gcc/c-family/c-warn.cc
> > +++ b/gcc/c-family/c-warn.cc
> > @@ -3029,7 +3029,7 @@ check_address_or_pointer_of_packed_member (tree type,
> > tree rhs)
> > if (TREE_CODE (rhs) == CALL_EXPR)
> > {
> >   rhs = CALL_EXPR_FN (rhs); /* Pointer expression.  */
> > - if (rhs == NULL_TREE)
> > + if (rhs == NULL_TREE || TREE_CODE (rhs) == IDENTIFIER_NODE)
> > return NULL_TREE;
> >   rhs = TREE_TYPE (rhs);/* Pointer type.  */
> >   /* We could be called while processing a template and RHS could be
> >  a functor.  In that case it's a class, not a pointer.  */
> >   if (!POINTER_TYPE_P (rhs))
> 
> How about adding !rhs to this condition instead of checking specifically for
> IDENTIFIER_NODE above?

Done.

> 
> > return NULL_TREE;
> 
> > @@ -1048,7 +1048,7 @@ maybe_warn_nodiscard (tree expr, impl_conv_void
> > implicit)
> >   call = TARGET_EXPR_INITIAL (expr);
> > location_t loc = cp_expr_loc_or_input_loc (call);
> > tree callee = cp_get_callee (call);
> > -  if (!callee)
> > +  if (!callee || identifier_p (callee))
> >

Re: [PATCH 1/2] c++: remove NON_DEPENDENT_EXPR, part 1

2023-09-26 Thread Jason Merrill

On 9/25/23 16:43, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?

-- >8 --

This tree code dates all the way back to r69130[1] which implemented
typing of non-dependent expressions.  Its motivation was never clear (to
me at least) since the documentation for it in e.g. cp-tree.def doesn't
seem accurate anymore.  build_non_dependent_expr has since gained
a bunch of edge cases about whether (or how) to wrap certain templated
trees, making it hard to reason about in general.

So this patch removes this tree code, and temporarily turns
build_non_dependent_expr into the identity function.  The subsequent
patch will remove build_non_dependent_expr and adjust its callers
appropriately.

We now need to gracefully handle templated (sub)trees in a couple of
places, places which previously didn't see templated trees since they
didn't look through NON_DEPENDENT_EXPR.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2003-July/109355.html

gcc/c-family/ChangeLog:

* c-warn.cc (check_address_or_pointer_of_packed_member): Handle
templated CALL_EXPR naming a local extern function.

gcc/cp/ChangeLog:

* class.cc (instantiate_type): Remove NON_DEPENDENT_EXPR
handling.
* constexpr.cc (cxx_eval_constant_expression): Likewise.
(potential_constant_expression_1): Likewise.
* coroutines.cc (coro_validate_builtin_call): Don't
expect ALIGNOF_EXPR to be wrapped in NON_DEPENDENT_EXPR.
* cp-objcp-common.cc (cp_common_init_ts): Remove
NON_DEPENDENT_EXPR handling.
* cp-tree.def (NON_DEPENDENT_EXPR): Remove.
* cp-tree.h (build_non_dependent_expr): Temporarily redefine as
the identity function.
* cvt.cc (maybe_warn_nodiscard): Handle templated CALL_EXPR
naming a local extern function.
* cxx-pretty-print.cc (cxx_pretty_printer::expression): Remove
NON_DEPENDENT_EXPR handling.
* error.cc (dump_decl): Likewise.
(dump_expr): Likewise.
* expr.cc (mark_use): Likewise.
(mark_exp_read): Likewise.
* pt.cc (build_non_dependent_expr): Remove.
* tree.cc (lvalue_kind): Remove NON_DEPENDENT_EXPR handling.
(cp_stabilize_reference): Likewise.
* typeck.cc (warn_for_null_address): Likewise.
(cp_build_binary_op): Handle type-dependent SIZEOF_EXPR operands.
(cp_build_unary_op) : Don't fold inside a
template.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/var-concept3.C: Adjust expected diagnostic
for attempting to call a variable concept.
---
  gcc/c-family/c-warn.cc   |  2 +-
  gcc/cp/class.cc  |  9 --
  gcc/cp/constexpr.cc  |  9 --
  gcc/cp/coroutines.cc |  3 +-
  gcc/cp/cp-objcp-common.cc|  1 -
  gcc/cp/cp-tree.def   | 11 ---
  gcc/cp/cp-tree.h |  2 +-
  gcc/cp/cvt.cc|  4 +-
  gcc/cp/cxx-pretty-print.cc   |  1 -
  gcc/cp/error.cc  |  8 --
  gcc/cp/expr.cc   |  2 -
  gcc/cp/pt.cc | 92 
  gcc/cp/tree.cc   |  5 --
  gcc/cp/typeck.cc | 13 +--
  gcc/testsuite/g++.dg/concepts/var-concept3.C |  2 +-
  15 files changed, 15 insertions(+), 149 deletions(-)

diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index e67dd87a773..c07770394bf 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -3029,7 +3029,7 @@ check_address_or_pointer_of_packed_member (tree type, 
tree rhs)
if (TREE_CODE (rhs) == CALL_EXPR)
{
  rhs = CALL_EXPR_FN (rhs); /* Pointer expression.  */
- if (rhs == NULL_TREE)
+ if (rhs == NULL_TREE || TREE_CODE (rhs) == IDENTIFIER_NODE)
return NULL_TREE;
  rhs = TREE_TYPE (rhs);/* Pointer type.  */
  /* We could be called while processing a template and RHS could be
 a functor.  In that case it's a class, not a pointer.  */
  if (!POINTER_TYPE_P (rhs))


How about adding !rhs to this condition instead of checking specifically 
for IDENTIFIER_NODE above?



return NULL_TREE;



@@ -1048,7 +1048,7 @@ maybe_warn_nodiscard (tree expr, impl_conv_void implicit)
  call = TARGET_EXPR_INITIAL (expr);
location_t loc = cp_expr_loc_or_input_loc (call);
tree callee = cp_get_callee (call);
-  if (!callee)
+  if (!callee || identifier_p (callee))
  return;


And similarly handling null type here?


@@ -5405,7 +5402,9 @@ cp_build_binary_op (const op_location_t ,
type0 = TREE_TYPE (type0);
  if (!TYPE_P (type1))
type1 = TREE_TYPE (type1);
- if (INDIRECT_TYPE_P (type0) && same_type_p (TREE_TYPE (type0), type1))
+ if 

[PATCH 1/2] c++: remove NON_DEPENDENT_EXPR, part 1

2023-09-25 Thread Patrick Palka
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk?

-- >8 --

This tree code dates all the way back to r69130[1] which implemented
typing of non-dependent expressions.  Its motivation was never clear (to
me at least) since the documentation for it in e.g. cp-tree.def doesn't
seem accurate anymore.  build_non_dependent_expr has since gained
a bunch of edge cases about whether (or how) to wrap certain templated
trees, making it hard to reason about in general.

So this patch removes this tree code, and temporarily turns
build_non_dependent_expr into the identity function.  The subsequent
patch will remove build_non_dependent_expr and adjust its callers
appropriately.

We now need to gracefully handle templated (sub)trees in a couple of
places, places which previously didn't see templated trees since they
didn't look through NON_DEPENDENT_EXPR.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2003-July/109355.html

gcc/c-family/ChangeLog:

* c-warn.cc (check_address_or_pointer_of_packed_member): Handle
templated CALL_EXPR naming a local extern function.

gcc/cp/ChangeLog:

* class.cc (instantiate_type): Remove NON_DEPENDENT_EXPR
handling.
* constexpr.cc (cxx_eval_constant_expression): Likewise.
(potential_constant_expression_1): Likewise.
* coroutines.cc (coro_validate_builtin_call): Don't
expect ALIGNOF_EXPR to be wrapped in NON_DEPENDENT_EXPR.
* cp-objcp-common.cc (cp_common_init_ts): Remove
NON_DEPENDENT_EXPR handling.
* cp-tree.def (NON_DEPENDENT_EXPR): Remove.
* cp-tree.h (build_non_dependent_expr): Temporarily redefine as
the identity function.
* cvt.cc (maybe_warn_nodiscard): Handle templated CALL_EXPR
naming a local extern function.
* cxx-pretty-print.cc (cxx_pretty_printer::expression): Remove
NON_DEPENDENT_EXPR handling.
* error.cc (dump_decl): Likewise.
(dump_expr): Likewise.
* expr.cc (mark_use): Likewise.
(mark_exp_read): Likewise.
* pt.cc (build_non_dependent_expr): Remove.
* tree.cc (lvalue_kind): Remove NON_DEPENDENT_EXPR handling.
(cp_stabilize_reference): Likewise.
* typeck.cc (warn_for_null_address): Likewise.
(cp_build_binary_op): Handle type-dependent SIZEOF_EXPR operands.
(cp_build_unary_op) : Don't fold inside a
template.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/var-concept3.C: Adjust expected diagnostic
for attempting to call a variable concept.
---
 gcc/c-family/c-warn.cc   |  2 +-
 gcc/cp/class.cc  |  9 --
 gcc/cp/constexpr.cc  |  9 --
 gcc/cp/coroutines.cc |  3 +-
 gcc/cp/cp-objcp-common.cc|  1 -
 gcc/cp/cp-tree.def   | 11 ---
 gcc/cp/cp-tree.h |  2 +-
 gcc/cp/cvt.cc|  4 +-
 gcc/cp/cxx-pretty-print.cc   |  1 -
 gcc/cp/error.cc  |  8 --
 gcc/cp/expr.cc   |  2 -
 gcc/cp/pt.cc | 92 
 gcc/cp/tree.cc   |  5 --
 gcc/cp/typeck.cc | 13 +--
 gcc/testsuite/g++.dg/concepts/var-concept3.C |  2 +-
 15 files changed, 15 insertions(+), 149 deletions(-)

diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index e67dd87a773..c07770394bf 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -3029,7 +3029,7 @@ check_address_or_pointer_of_packed_member (tree type, 
tree rhs)
   if (TREE_CODE (rhs) == CALL_EXPR)
{
  rhs = CALL_EXPR_FN (rhs); /* Pointer expression.  */
- if (rhs == NULL_TREE)
+ if (rhs == NULL_TREE || TREE_CODE (rhs) == IDENTIFIER_NODE)
return NULL_TREE;
  rhs = TREE_TYPE (rhs);/* Pointer type.  */
  /* We could be called while processing a template and RHS could be
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index b71333af1f8..10de0437242 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -8843,15 +8843,6 @@ instantiate_type (tree lhstype, tree rhs, tsubst_flags_t 
complain)
   rhs = BASELINK_FUNCTIONS (rhs);
 }
 
-  /* If we are in a template, and have a NON_DEPENDENT_EXPR, we cannot
- deduce any type information.  */
-  if (TREE_CODE (rhs) == NON_DEPENDENT_EXPR)
-{
-  if (complain & tf_error)
-   error ("not enough type information");
-  return error_mark_node;
-}
-
   /* There are only a few kinds of expressions that may have a type
  dependent on overload resolution.  */
   gcc_assert (TREE_CODE (rhs) == ADDR_EXPR
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 2a6601c0cbc..8c9abeeec1b 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8054,7 +8054,6 @@