On Mon, 2018-01-08 at 16:59 -0500, Jason Merrill wrote:
> On 01/08/2018 04:49 PM, Jason Merrill wrote:
> > On 01/08/2018 04:01 PM, David Malcolm wrote:
> > > On Fri, 2018-01-05 at 15:29 -0500, Jason Merrill wrote:
> > > >
> > > > I'd rather handle location wrappers separately, and abort if
> > > > VIEW_CONVERT_EXPR or NON_LVALUE_EXPR appear other than as
> > > > wrappers.
> > >
> > > Once I fixed the issue with location_wrapper_p with decls
> > > changing
> > > type, it turns out that trunk is already passing
> > > VIEW_CONVERT_EXPR to
> > > tsubst_copy_and_build for non-wrapper nodes (and from there to
> > > tsubst_copy), where the default case currently handles them.
> > > Adding an
> > > assert turns this into an ICE.
> > >
> > > g++.dg/delayedfold/builtin1.C is the only instance of it I found
> > > in our
> > > test suite, where it's used here:
> > >
> > > class RegionLock {
> > > template <unsigned long> void m_fn1();
> > > int spinlock;
> > > } acquire_zero;
> > > int acquire_one;
> > > template <unsigned long> void RegionLock::m_fn1() {
> > > __atomic_compare_exchange(&spinlock, &acquire_zero,
> > > &acquire_one,
> > > false, 2, 2);
> > > ^~~~~~~~~~~~~
> > > }
> > >
> > > (gdb) call debug_tree (t)
> > > <view_convert_expr 0x7ffff1a15b40
> >
> > ...
> > > arg:0 <non_dependent_expr 0x7ffff1a15aa0
> >
> > ...
> > > arg:0 <addr_expr 0x7ffff1a159e0 type <pointer_type
> > > 0x7ffff1a18150>
> > > arg:0 <var_decl 0x7ffff7ffbd80 acquire_zero>
> > >
> > > (This one is just for VIEW_CONVERT_EXPR; I don't yet know of any
> > > existing places where NON_LVALUE_EXPR can be passed to tsubst_*).
> >
> > Hmm, a NON_DEPENDENT_EXPR also shouldn't make it into the saved
> > trees
> > for a template. I'll take a look.
>
> OK, we're only seeing this when args is NULL_TREE (i.e. under
> instantiate_non_dependent_expr), so tsubst_copy immediately returns.
> The assert should come after that.
>
> Jason
Aha - thanks!
Here's an updated version of the patch. It adds VIEW_CONVERT_EXPR and
NON_LVALUE_EXPR to tsubst_copy, but only supports them there for location
wrapper nodes, with an assertion. It also adds them to
tsubst_copy_and_build, with a suitable assertion to handle the case
described above.
I also updated build_non_dependent_expr to avoid losing location wrappers
(by capturing the "orig_expr" and return it rather than "expr").
Successfully bootstrapped®rtested 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?
(this version of the patch requires the updated version of the [02/14]
patch I posted here:
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00591.html
to handle the types of decls from changing under a wrapper,
which is the only other part of the kit still needing review).
Dave
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 | 85 +++++++++++++++++++++++++++++++++++++++++++++++-----
6 files changed, 111 insertions(+), 16 deletions(-)
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 934f674..9bdaafc 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2058,7 +2058,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. */
@@ -2075,8 +2075,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 805319a..2c53740 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 d4b45a4..afb4813 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 ffd2b4c..895269a 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -2889,6 +2889,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 b04ed9a..1506ea4 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 a8144e8..392fd7f 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 build1 (code, TREE_TYPE (op0), op0);
+ }
+
case CAST_EXPR:
case REINTERPRET_CAST_EXPR:
case CONST_CAST_EXPR:
@@ -18290,6 +18303,18 @@ 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 for
+ VIEW_CONVERT_EXPRs within instantiate_non_dependent_expr (when
+ args is NULL_TREE). */
+ gcc_assert (location_wrapper_p (t)
+ || (TREE_CODE (t) == VIEW_CONVERT_EXPR
+ && args == NULL_TREE));
+ if (location_wrapper_p (t))
+ RETURN (RECUR (TREE_OPERAND (t, 0)));
+ /* fallthrough. */
+
default:
/* Handle Objective-C++ constructs, if appropriate. */
{
@@ -24981,6 +25006,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 +25023,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 +25036,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 +26628,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
