[PATCH v4 of 03/14] C++: add location_t wrapper nodes during parsing (minimal impl)

[David Malcolm <dmalcolm at redhat dot com>]

On Tue, 2018-01-09 at 15:39 +0100, Jakub Jelinek wrote:
> On Tue, Jan 09, 2018 at 09:36:58AM -0500, Jason Merrill wrote:
> > On 01/09/2018 06:53 AM, David Malcolm wrote:
> > > +    case NON_LVALUE_EXPR:
> > > +    case VIEW_CONVERT_EXPR:
> > > +	{
> > > +	  /* Handle location wrappers by substituting the
> > > wrapped node
> > > +	     first,*then*  reusing the resulting type.  Doing
> > > the type
> > > +	     first ensures that we handle template parameters
> > > and
> > > +	     parameter pack expansions.  */
> > > +	  gcc_assert (location_wrapper_p (t));
> > > +	  tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args,
> > > complain, in_decl);
> > > +	  return build1 (code, TREE_TYPE (op0), op0);
> > > +	}
> > 
> > Doesn't this lose the location information?
> 
> And the public_flag...
> 
> 	Jakub

Ooops, yes.  Thanks.  I'm not convinced we always retain location
information in the tsubst_* calls: although we override input_location
within them, I see lots of pre-existing "build1" calls (as opposed to
"build1_loc"), which as I understand it set any EXPR_LOCATION to be
UNKNOWN_LOCATION.  On the other hand, even if I'm correct, that feels
like a pre-existing issue and orthogonal to this patch kit.

Here's an updated version of the patch which uses maybe_wrap_with_location
in tsubst_copy and tsubst_copy_and_build when copying the wrappers
(thus setting the flag, but hiding it as an implementation detail
within maybe_wrap_with_location).

I also updated the assertion as per Jason's other comment
(re NON_LVALUE_EXPR when args is NULL_TREE).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu, as
part of the kit.
Also, manually tested with "make s-selftest-c++" (since we don't run
the selftests for cc1plus by default).

OK for trunk in conjunction with the rest of the kit?

gcc/cp/ChangeLog:
	* cp-gimplify.c (cp_fold): Remove the early bailout when
	processing_template_decl.
	* cp-lang.c (selftest::run_cp_tests): Call
	selftest::cp_pt_c_tests.
	* cp-tree.h (selftest::cp_pt_c_tests): New decl.
	* mangle.c (write_expression): Skip location wrapper nodes.
	* parser.c (cp_parser_postfix_expression): Call
	maybe_add_location_wrapper on the result for RID_TYPEID. Pass true
	for new "wrap_locations_p" param of
	cp_parser_parenthesized_expression_list.
	(cp_parser_parenthesized_expression_list): Add "wrap_locations_p"
	param, defaulting to false.  Convert "expr" to a cp_expr, and call
	maybe_add_location_wrapper on it when wrap_locations_p is true.
	(cp_parser_unary_expression): Call maybe_add_location_wrapper on
	the result for RID_ALIGNOF and RID_SIZEOF.
	(cp_parser_builtin_offsetof): Likewise.
	* pt.c: Include "selftest.h".
	(tsubst_copy): Handle location wrappers.
	(tsubst_copy_and_build): Likewise.
	(build_non_dependent_expr): Likewise.
	(selftest::test_build_non_dependent_expr): New function.
	(selftest::cp_pt_c_tests): New function.
---
 gcc/cp/cp-gimplify.c |  5 ++--
 gcc/cp/cp-lang.c     |  1 +
 gcc/cp/cp-tree.h     | 10 +++++++
 gcc/cp/mangle.c      |  1 +
 gcc/cp/parser.c      | 25 ++++++++++++----
 gcc/cp/pt.c          | 83 +++++++++++++++++++++++++++++++++++++++++++++++-----
 6 files changed, 109 insertions(+), 16 deletions(-)

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index eda493a..e97247c 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2064,7 +2064,7 @@ clear_fold_cache (void)
 
 /*  This function tries to fold an expression X.
     To avoid combinatorial explosion, folding results are kept in fold_cache.
-    If we are processing a template or X is invalid, we don't fold at all.
+    If X is invalid, we don't fold at all.
     For performance reasons we don't cache expressions representing a
     declaration or constant.
     Function returns X or its folded variant.  */
@@ -2081,8 +2081,7 @@ cp_fold (tree x)
   if (!x || x == error_mark_node)
     return x;
 
-  if (processing_template_decl
-      || (EXPR_P (x) && (!TREE_TYPE (x) || TREE_TYPE (x) == error_mark_node)))
+  if (EXPR_P (x) && (!TREE_TYPE (x) || TREE_TYPE (x) == error_mark_node))
     return x;
 
   /* Don't bother to cache DECLs or constants.  */
diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index 9992bc2..1ce986e 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -247,6 +247,7 @@ run_cp_tests (void)
   c_family_tests ();
 
   /* Additional C++-specific tests.  */
+  cp_pt_c_tests ();
 }
 
 } // namespace selftest
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index e0ce14e..901493f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7464,6 +7464,16 @@ named_decl_hash::equal (const value_type existing, compare_type candidate)
   return candidate == name;
 }
 
+#if CHECKING_P
+namespace selftest {
+  extern void run_cp_tests (void);
+
+  /* Declarations for specific families of tests within cp,
+     by source file, in alphabetical order.  */
+  extern void cp_pt_c_tests ();
+} // namespace selftest
+#endif /* #if CHECKING_P */
+
 /* -- end of C++ */
 
 #endif /* ! GCC_CP_TREE_H */
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index bd74543..94c4bed 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -2890,6 +2890,7 @@ write_expression (tree expr)
   /* Skip NOP_EXPR and CONVERT_EXPR.  They can occur when (say) a pointer
      argument is converted (via qualification conversions) to another type.  */
   while (CONVERT_EXPR_CODE_P (code)
+	 || location_wrapper_p (expr)
 	 /* Parentheses aren't mangled.  */
 	 || code == PAREN_EXPR
 	 || code == NON_LVALUE_EXPR)
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f9181b7..e31d103 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -2047,7 +2047,8 @@ static tree cp_parser_postfix_open_square_expression
 static tree cp_parser_postfix_dot_deref_expression
   (cp_parser *, enum cpp_ttype, cp_expr, bool, cp_id_kind *, location_t);
 static vec<tree, va_gc> *cp_parser_parenthesized_expression_list
-  (cp_parser *, int, bool, bool, bool *, location_t * = NULL);
+  (cp_parser *, int, bool, bool, bool *, location_t * = NULL,
+   bool = false);
 /* Values for the second parameter of cp_parser_parenthesized_expression_list.  */
 enum { non_attr = 0, normal_attr = 1, id_attr = 2 };
 static void cp_parser_pseudo_destructor_name
@@ -6831,6 +6832,7 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
 	    location_t typeid_loc
 	      = make_location (start_loc, start_loc, close_paren->location);
 	    postfix_expression.set_location (typeid_loc);
+	    postfix_expression.maybe_add_location_wrapper ();
 	  }
       }
       break;
@@ -7088,7 +7090,8 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
 		    (parser, non_attr,
 		     /*cast_p=*/false, /*allow_expansion_p=*/true,
 		     /*non_constant_p=*/NULL,
-		     /*close_paren_loc=*/&close_paren_loc));
+		     /*close_paren_loc=*/&close_paren_loc,
+		     /*wrap_locations_p=*/true));
 	    if (is_builtin_constant_p)
 	      {
 		parser->integral_constant_expression_p
@@ -7621,6 +7624,10 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
    ALLOW_EXPANSION_P is true if this expression allows expansion of an
    argument pack.
 
+   WRAP_LOCATIONS_P is true if expressions within this list for which
+   CAN_HAVE_LOCATION_P is false should be wrapped with nodes expressing
+   their source locations.
+
    Returns a vector of trees.  Each element is a representation of an
    assignment-expression.  NULL is returned if the ( and or ) are
    missing.  An empty, but allocated, vector is returned on no
@@ -7640,7 +7647,8 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
 					 bool cast_p,
                                          bool allow_expansion_p,
 					 bool *non_constant_p,
-					 location_t *close_paren_loc)
+					 location_t *close_paren_loc,
+					 bool wrap_locations_p)
 {
   vec<tree, va_gc> *expression_list;
   bool fold_expr_p = is_attribute_list != non_attr;
@@ -7663,12 +7671,12 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
     = parser->greater_than_is_operator_p;
   parser->greater_than_is_operator_p = true;
 
+  cp_expr expr (NULL_TREE);
+
   /* Consume expressions until there are no more.  */
   if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN))
     while (true)
       {
-	tree expr;
-
 	/* At the beginning of attribute lists, check to see if the
 	   next token is an identifier.  */
 	if (is_attribute_list == id_attr
@@ -7722,11 +7730,14 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
                 expr = make_pack_expansion (expr);
               }
 
+	    if (wrap_locations_p)
+	      expr.maybe_add_location_wrapper ();
+
 	     /* Add it to the list.  We add error_mark_node
 		expressions to the list, so that we can still tell if
 		the correct form for a parenthesized expression-list
 		is found. That gives better errors.  */
-	    vec_safe_push (expression_list, expr);
+	    vec_safe_push (expression_list, expr.get_value ());
 
 	    if (expr == error_mark_node)
 	      goto skip_comma;
@@ -7992,6 +8003,7 @@ cp_parser_unary_expression (cp_parser *parser, cp_id_kind * pidk,
 
 	    cp_expr ret_expr (ret);
 	    ret_expr.set_location (compound_loc);
+	    ret_expr = ret_expr.maybe_add_location_wrapper ();
 	    return ret_expr;
 	  }
 
@@ -9831,6 +9843,7 @@ cp_parser_builtin_offsetof (cp_parser *parser)
   parser->integral_constant_expression_p = save_ice_p;
   parser->non_integral_constant_expression_p = save_non_ice_p;
 
+  expr = expr.maybe_add_location_wrapper ();
   return expr;
 }
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 2fb327a..4f8086b 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "type-utils.h"
 #include "gimplify.h"
 #include "gcc-rich-location.h"
+#include "selftest.h"
 
 /* The type of functions taking a tree, and some additional data, and
    returning an int.  */
@@ -14924,6 +14925,18 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	/* Ordinary template template argument.  */
 	return t;
 
+    case NON_LVALUE_EXPR:
+    case VIEW_CONVERT_EXPR:
+	{
+	  /* Handle location wrappers by substituting the wrapped node
+	     first, *then* reusing the resulting type.  Doing the type
+	     first ensures that we handle template parameters and
+	     parameter pack expansions.  */
+	  gcc_assert (location_wrapper_p (t));
+	  tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, complain, in_decl);
+	  return maybe_wrap_with_location (op0, EXPR_LOCATION (t));
+	}
+
     case CAST_EXPR:
     case REINTERPRET_CAST_EXPR:
     case CONST_CAST_EXPR:
@@ -18290,6 +18303,16 @@ tsubst_copy_and_build (tree t,
     case REQUIRES_EXPR:
       RETURN (tsubst_requires_expr (t, args, complain, in_decl));
 
+    case NON_LVALUE_EXPR:
+    case VIEW_CONVERT_EXPR:
+      /* We should only see these for location wrapper nodes, or within
+	 instantiate_non_dependent_expr (when args is NULL_TREE).  */
+      gcc_assert (location_wrapper_p (t) || args == NULL_TREE);
+      if (location_wrapper_p (t))
+	RETURN (maybe_wrap_with_location (RECUR (TREE_OPERAND (t, 0)),
+					  EXPR_LOCATION (t)));
+      /* fallthrough.  */
+
     default:
       /* Handle Objective-C++ constructs, if appropriate.  */
       {
@@ -24981,6 +25004,7 @@ resolve_typename_type (tree type, bool only_current_p)
 tree
 build_non_dependent_expr (tree expr)
 {
+  tree orig_expr = expr;
   tree inner_expr;
 
   /* When checking, try to get a constant value for all non-dependent
@@ -24997,6 +25021,8 @@ build_non_dependent_expr (tree expr)
       && !expanding_concept ())
     fold_non_dependent_expr (expr);
 
+  STRIP_ANY_LOCATION_WRAPPER (expr);
+
   /* Preserve OVERLOADs; the functions must be available to resolve
      types.  */
   inner_expr = expr;
@@ -25008,36 +25034,36 @@ build_non_dependent_expr (tree expr)
     inner_expr = TREE_OPERAND (inner_expr, 1);
   if (is_overloaded_fn (inner_expr)
       || TREE_CODE (inner_expr) == OFFSET_REF)
-    return expr;
+    return orig_expr;
   /* There is no need to return a proxy for a variable.  */
   if (VAR_P (expr))
-    return expr;
+    return orig_expr;
   /* Preserve string constants; conversions from string constants to
      "char *" are allowed, even though normally a "const char *"
      cannot be used to initialize a "char *".  */
   if (TREE_CODE (expr) == STRING_CST)
-    return expr;
+    return orig_expr;
   /* Preserve void and arithmetic constants, as an optimization -- there is no
      reason to create a new node.  */
   if (TREE_CODE (expr) == VOID_CST
       || TREE_CODE (expr) == INTEGER_CST
       || TREE_CODE (expr) == REAL_CST)
-    return expr;
+    return orig_expr;
   /* Preserve THROW_EXPRs -- all throw-expressions have type "void".
      There is at least one place where we want to know that a
      particular expression is a throw-expression: when checking a ?:
      expression, there are special rules if the second or third
      argument is a throw-expression.  */
   if (TREE_CODE (expr) == THROW_EXPR)
-    return expr;
+    return orig_expr;
 
   /* Don't wrap an initializer list, we need to be able to look inside.  */
   if (BRACE_ENCLOSED_INITIALIZER_P (expr))
-    return expr;
+    return orig_expr;
 
   /* Don't wrap a dummy object, we need to be able to test for it.  */
   if (is_dummy_object (expr))
-    return expr;
+    return orig_expr;
 
   if (TREE_CODE (expr) == COND_EXPR)
     return build3 (COND_EXPR,
@@ -26600,4 +26626,47 @@ print_template_statistics (void)
 	   type_specializations->collisions ());
 }
 
+#if CHECKING_P
+
+namespace selftest {
+
+/* Verify that build_non_dependent_expr () works, for various expressions,
+   and that location wrappers don't affect the results.  */
+
+static void
+test_build_non_dependent_expr ()
+{
+  location_t loc = BUILTINS_LOCATION;
+
+  /* Verify constants, without and with location wrappers.  */
+  tree int_cst = build_int_cst (integer_type_node, 42);
+  ASSERT_EQ (int_cst, build_non_dependent_expr (int_cst));
+
+  tree wrapped_int_cst = maybe_wrap_with_location (int_cst, loc);
+  ASSERT_TRUE (location_wrapper_p (wrapped_int_cst));
+  ASSERT_EQ (wrapped_int_cst, build_non_dependent_expr (wrapped_int_cst));
+
+  tree string_lit = build_string (4, "foo");
+  TREE_TYPE (string_lit) = char_array_type_node;
+  string_lit = fix_string_type (string_lit);
+  ASSERT_EQ (string_lit, build_non_dependent_expr (string_lit));
+
+  tree wrapped_string_lit = maybe_wrap_with_location (string_lit, loc);
+  ASSERT_TRUE (location_wrapper_p (wrapped_string_lit));
+  ASSERT_EQ (wrapped_string_lit,
+	     build_non_dependent_expr (wrapped_string_lit));
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+cp_pt_c_tests ()
+{
+  test_build_non_dependent_expr ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
+
 #include "gt-cp-pt.h"
-- 
1.8.5.3