This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH/RFC] C++ FE: expression ranges (v2)
- From: Jason Merrill <jason at redhat dot com>
- To: David Malcolm <dmalcolm at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Sat, 21 Nov 2015 02:16:49 -0500
- Subject: Re: [PATCH/RFC] C++ FE: expression ranges (v2)
- Authentication-results: sourceware.org; auth=none
- References: <1446868737-3306-1-git-send-email-dmalcolm at redhat dot com> <1447563717-24429-1-git-send-email-dmalcolm at redhat dot com> <564E3512 dot 9080701 at redhat dot com>
On 11/19/2015 03:46 PM, Jason Merrill wrote:
On 11/15/2015 12:01 AM, David Malcolm wrote:
As with the C frontend, there's an issue with tree nodes that
don't have locations: VAR_DECL, INTEGER_CST, etc:
int test (int foo)
{
return foo * 100;
^^^ ^^^
}
where we'd like to access the source spelling ranges of the expressions
during parsing, so that we can use them when reporting parser errors.
Hmm, I had been thinking to address this in the C++ front end by
wrapping uses in another tree: NOP_EXPR for rvalues, VIEW_CONVERT_EXPR
for lvalues.
On the other hand, my direction seems likely to cause more issues,
especially with code that doesn't yet know how to handle
VIEW_CONVERT_EXPR, and could create ambiguity with explicit conversions.
So I guess your approach seems reasonable.
What is the memory consumption impact of this change?
Also, in cp_parser_new_expression I attempted to generate meaningful
ranges e.g.:
int *foo = new int[100];
^~~~~~~~~~~~
but it seems to be hard to do this, due to the trailing optional
components; I found myself wanting to ask the lexer for the last
token it consumed (in particular, I'm interested in the
location_t of that token). Is this a reasonable thing to add to the
lexer?
cp_lexer_previous_token seems like what you want.
- return cp_build_unary_op (code, arg1, candidates != 0, complain);
+ {
+ tree result = cp_build_unary_op (code, arg1, candidates != 0, complain);
+ protected_set_expr_location (result, loc);
I'd much rather pass loc into cp_build_unary_op. At least add a FIXME
to that effect. Likewise in the other places you call *build* and then
set the loc.
+#if 0
+ /* FIXME: various assertions can be put in here when debugging,
+ for tracking down where location information gets thrown
+ away (during a trip through a purely "tree" value). */
+ if (m_value && m_value != error_mark_node)
+ {
+ if (TREE_CODE (m_value) == FUNCTION_DECL)
+ return; // for now
+ gcc_assert (CAN_HAVE_LOCATION_P (m_value));
+ //gcc_assert (m_loc != UNKNOWN_LOCATION);
Yeah, I don't think you want to assert on UNKNOWN_LOCATION; some code
does not and should not have an associated location. In particular,
cleanups.
Build the assignment expression. Its default
- location is the location of the '=' token. */
+ location:
+ LHS = RHS
+ ~~~~^~~~~
+ is the location of the '=' token as the
+ caret, ranging from the start of the lhs to the
+ end of the rhs. */
saved_input_location = input_location;
+ loc = make_location (loc,
+ expr.get_start (),
+ rhs.get_finish ());
input_location = loc;
expr = build_x_modify_expr (loc, expr,
assignment_operator,
rhs,
complain_flags (decltype_p));
+ protected_set_expr_location (expr, loc);
input_location = saved_input_location;
Do we still need to mess with input_location here? If so, please add a
FIXME explaining what still needs to be fixed.
Jason