Hi,
On Thu, Jul 29, 2021 at 05:18:50PM +0200, Thomas Schwinge wrote:
> On 2021-07-29T12:55:38+0200, Mark Wielaard wrote:
> > On Thu, 2021-07-29 at 09:25 +0800, The Other via Gcc-rust wrote:
> >> But isn’t it overkill to pass the token in instead of just the
> >> location? You can avoid a fairly expensive shared_ptr copy by doing
> >> so.
> >
> > I hadn't even noticed it was a smart pointer. At least the abstraction
> > works transparently.
>
> Wouldn't 'const_TokenPtr &pratt_parsed_token' work, passing a C++
> reference? (Or am I missing some C++ "detail"; I haven't looked
> carefully.)
I think the issue is not the passing of the actual smart pointer, but
the construction in the default case. When we define the method
argument as const_TokenPtr pratt_parsed_token = nullptr we are
actually constructing a shared_ptr wrapper around the nullptr.
Of course in the case of passing a Location we also always have to
construct an object. But the Location class is very simple, so
hopefully it is cheaper to construct.
> > But yes, things might be even simpler by passing the location directly.
> > I'll try to rewrite the code tonight to pass a location and see how
> > that looks.
That variant is attached and can also be found here:
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-loc
The original is also here:
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-token
Hopefully one of the two is acceptable. If not please let me know how
to rewrite it in a cheaper more idiomatic way.
Thanks,
Mark>From 2c51713eec25b38ab2274537103a391d89894dba Mon Sep 17 00:00:00 2001
From: Mark Wielaard
Date: Thu, 29 Jul 2021 00:00:55 +0200
Subject: [PATCH] Pass pratt parsed location to expr parser functions to fix
expr locus
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The pratt parser skips the first token of an expression before
invoking the actual expression parsing function. This makes getting
the correct starting location of a pratt parsed expression hard. The
"correction" of the location by subtracting an integer is often wrong
(since there may be arbitrary whitespace or comments between
tokens). Fix this by passing the location of the skipped token to the
expression parsing functions (instead of just providing a pratt_parse
boolean). Use this location to set the start of the expression.
Before gccrs would generate the following error message:
return.rs:3:22: error: cannot ‘break’ outside of a loop
3 | let x = 5 - break return (16 + 2);
| ^
Now we get:
return.rs:3:17: error: cannot ‘break’ outside of a loop
3 | let x = 5 - break return (16 + 2);
| ^
---
gcc/rust/parse/rust-parse-impl.h | 177 +++
gcc/rust/parse/rust-parse.h | 63 ++-
2 files changed, 95 insertions(+), 145 deletions(-)
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index 340fea70201..4b208f0da23 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -6580,7 +6580,7 @@ Parser::parse_path_expr_segment ()
template
AST::QualifiedPathInExpression
Parser::parse_qualified_path_in_expression (
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
/* Note: the Rust grammar is defined in such a way that it is impossible to
* determine whether a prospective qualified path is a
@@ -6595,7 +6595,7 @@ Parser::parse_qualified_path_in_expression (
// parse the qualified path type (required)
AST::QualifiedPathType qual_path_type
-= parse_qualified_path_type (pratt_parse);
+= parse_qualified_path_type (pratt_parsed_loc);
if (qual_path_type.is_error ())
{
// TODO: should this create a parse error?
@@ -6661,12 +6661,13 @@ Parser::parse_qualified_path_in_expression (
// Parses the type syntactical construction at the start of a qualified path.
template
AST::QualifiedPathType
-Parser::parse_qualified_path_type (bool pratt_parse)
+Parser::parse_qualified_path_type (
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
+ Location locus = pratt_parsed_loc;
/* TODO: should this actually be error? is there anywhere where this could be
* valid? */
- if (!pratt_parse)
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_ANGLE))
@@ -6675,11 +6676,6 @@ Parser::parse_qualified_path_type (bool pratt_parse)
return AST::QualifiedPathType::create_error ();
}
}
- else
-{
- // move back by 1 if pratt parsing due to skipping '<'
- locus = lexer.peek_token ()->get_locus () - 1;
-}
// parse type (required)
std::unique_ptr type = parse_type ();
@@ -7305,10 +7301,10 @@ Parser::parse_expr_without_block (AST::AttrVec outer_attrs)
template
std::unique_ptr
Parser::parse_block_expr (AST::AttrVec outer_attrs,
-
