* [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
@ 2021-07-28 22:13 Mark Wielaard
2021-07-29 1:25 ` The Other
0 siblings, 1 reply; 10+ messages in thread
From: Mark Wielaard @ 2021-07-28 22:13 UTC (permalink / raw)
To: gcc-rust; +Cc: Mark Wielaard
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 token to the expression parsing
functions (instead of just providing a pratt_parse boolean). Use this
token 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 | 180 +++++++++++++------------------
gcc/rust/parse/rust-parse.h | 58 +++++-----
2 files changed, 103 insertions(+), 135 deletions(-)
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index 340fea70201..6241a972ff7 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -6580,7 +6580,7 @@ Parser<ManagedTokenSource>::parse_path_expr_segment ()
template <typename ManagedTokenSource>
AST::QualifiedPathInExpression
Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
/* 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<ManagedTokenSource>::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_token);
if (qual_path_type.is_error ())
{
// TODO: should this create a parse error?
@@ -6661,12 +6661,13 @@ Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
// Parses the type syntactical construction at the start of a qualified path.
template <typename ManagedTokenSource>
AST::QualifiedPathType
-Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
+Parser<ManagedTokenSource>::parse_qualified_path_type (
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
+ Location locus;
/* TODO: should this actually be error? is there anywhere where this could be
* valid? */
- if (!pratt_parse)
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_ANGLE))
@@ -6676,10 +6677,7 @@ Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
}
}
else
- {
- // move back by 1 if pratt parsing due to skipping '<'
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse type (required)
std::unique_ptr<AST::Type> type = parse_type ();
@@ -7305,10 +7303,10 @@ Parser<ManagedTokenSource>::parse_expr_without_block (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::BlockExpr>
Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_CURLY))
@@ -7318,9 +7316,7 @@ Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
}
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -7611,22 +7607,17 @@ Parser<ManagedTokenSource>::parse_literal_expr (AST::AttrVec outer_attrs)
// Parses a return expression (including any expression to return).
template <typename ManagedTokenSource>
std::unique_ptr<AST::ReturnExpr>
-Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_return_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (RETURN_TOK);
}
else
- {
- // minus 7 chars for 6 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 7;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse expression to return, if it exists
ParseRestrictions restrictions;
@@ -7644,21 +7635,16 @@ Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::BreakExpr>
Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (BREAK);
}
else
- {
- // minus 6 chars for 5 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 6;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7681,22 +7667,17 @@ Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
// Parses a continue expression (including any label to continue from).
template <typename ManagedTokenSource>
std::unique_ptr<AST::ContinueExpr>
-Parser<ManagedTokenSource>::parse_continue_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_continue_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (CONTINUE);
}
else
- {
- // minus 9 chars for 8 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 9;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7740,11 +7721,11 @@ Parser<ManagedTokenSource>::parse_loop_label ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfExpr>
Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7754,9 +7735,7 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
}
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// detect accidental if let
if (lexer.peek_token ()->get_id () == LET)
@@ -7901,12 +7880,12 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
* expressions don't support them. */
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfLetExpr>
-Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_if_let_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7916,9 +7895,7 @@ Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
}
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// detect accidental if expr parsed as if let expr
if (lexer.peek_token ()->get_id () != LET)
@@ -8094,10 +8071,10 @@ template <typename ManagedTokenSource>
std::unique_ptr<AST::LoopExpr>
Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
AST::LoopLabel label,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8113,7 +8090,7 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
else
{
if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
+ locus = pratt_parsed_token->get_locus ();
else
locus = label.get_locus ();
}
@@ -8138,12 +8115,12 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
* via parse_labelled_loop_expr, which would call this. */
template <typename ManagedTokenSource>
std::unique_ptr<AST::WhileLoopExpr>
-Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
- AST::LoopLabel label,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_while_loop_expr (
+ AST::AttrVec outer_attrs, AST::LoopLabel label,
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8159,7 +8136,7 @@ Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
else
{
if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
+ locus = pratt_parsed_token->get_locus ();
else
locus = label.get_locus ();
}
@@ -8417,21 +8394,16 @@ Parser<ManagedTokenSource>::parse_labelled_loop_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::MatchExpr>
Parser<ManagedTokenSource>::parse_match_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (MATCH_TOK);
}
else
- {
- // TODO: probably just pass in location data as param
- // get current pos then move back 6 - 5 for match, 1 for space
- locus = lexer.peek_token ()->get_locus () - 6;
- }
+ locus = pratt_parsed_token->get_locus ();
/* parse scrutinee expression, which is required (and HACK to prevent struct
* expr) */
@@ -8704,17 +8676,17 @@ Parser<ManagedTokenSource>::parse_async_block_expr (AST::AttrVec outer_attrs)
// Parses an unsafe block expression.
template <typename ManagedTokenSource>
std::unique_ptr<AST::UnsafeBlockExpr>
-Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_unsafe_block_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
Location locus;
- if (!pratt_parse)
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
skip_token (UNSAFE);
}
else
- locus = lexer.peek_token ()->get_locus () - 1;
+ locus = pratt_parsed_token->get_locus ();
// parse block expression (required)
std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
@@ -8738,19 +8710,16 @@ Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::ArrayExpr>
Parser<ManagedTokenSource>::parse_array_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_SQUARE);
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -8927,20 +8896,17 @@ Parser<ManagedTokenSource>::parse_closure_param ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::ExprWithoutBlock>
Parser<ManagedTokenSource>::parse_grouped_or_tuple_expr (
- AST::AttrVec outer_attrs, bool pratt_parse)
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
// adjustment to allow Pratt parsing to reuse function without copy-paste
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_PAREN);
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -12535,7 +12501,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
// qualified path
// HACK: add outer attrs to path
AST::QualifiedPathInExpression path
- = parse_qualified_path_in_expression (true);
+ = parse_qualified_path_in_expression (tok);
path.set_outer_attrs (std::move (outer_attrs));
return std::unique_ptr<AST::QualifiedPathInExpression> (
new AST::QualifiedPathInExpression (std::move (path)));
@@ -12578,7 +12544,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
new AST::LiteralExpr ("false", AST::Literal::BOOL,
tok->get_type_hint (), {}, tok->get_locus ()));
case LEFT_PAREN:
- return parse_grouped_or_tuple_expr (std::move (outer_attrs), true);
+ return parse_grouped_or_tuple_expr (std::move (outer_attrs), tok);
/*case PLUS: { // unary plus operator
// invoke parse_expr recursively with appropriate priority, etc. for
@@ -12810,41 +12776,41 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
return parse_range_to_inclusive_expr (tok, std::move (outer_attrs));
case RETURN_TOK:
// FIXME: is this really a null denotation expression?
- return parse_return_expr (std::move (outer_attrs), true);
+ return parse_return_expr (std::move (outer_attrs), tok);
case BREAK:
// FIXME: is this really a null denotation expression?
- return parse_break_expr (std::move (outer_attrs), true);
+ return parse_break_expr (std::move (outer_attrs), tok);
case CONTINUE:
- return parse_continue_expr (std::move (outer_attrs), true);
+ return parse_continue_expr (std::move (outer_attrs), tok);
case LEFT_CURLY:
// ok - this is an expression with block for once.
- return parse_block_expr (std::move (outer_attrs), true);
+ return parse_block_expr (std::move (outer_attrs), tok);
case IF:
// if or if let, so more lookahead to find out
if (lexer.peek_token (1)->get_id () == LET)
{
// if let expr
- return parse_if_let_expr (std::move (outer_attrs), true);
+ return parse_if_let_expr (std::move (outer_attrs), tok);
}
else
{
// if expr
- return parse_if_expr (std::move (outer_attrs), true);
+ return parse_if_expr (std::move (outer_attrs), tok);
}
case LOOP:
return parse_loop_expr (std::move (outer_attrs), AST::LoopLabel::error (),
- true);
+ tok);
case WHILE:
return parse_while_loop_expr (std::move (outer_attrs),
- AST::LoopLabel::error (), true);
+ AST::LoopLabel::error (), tok);
case MATCH_TOK:
// also an expression with block
- return parse_match_expr (std::move (outer_attrs), true);
+ return parse_match_expr (std::move (outer_attrs), tok);
case LEFT_SQUARE:
// array definition expr (not indexing)
- return parse_array_expr (std::move (outer_attrs), true);
+ return parse_array_expr (std::move (outer_attrs), tok);
case UNSAFE:
- return parse_unsafe_block_expr (std::move (outer_attrs), true);
+ return parse_unsafe_block_expr (std::move (outer_attrs), tok);
default:
if (!restrictions.expr_can_be_null)
add_error (Error (tok->get_locus (),
diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
index 1c7bd781b3f..78608839265 100644
--- a/gcc/rust/parse/rust-parse.h
+++ b/gcc/rust/parse/rust-parse.h
@@ -121,8 +121,10 @@ private:
AST::PathInExpression parse_path_in_expression ();
AST::PathExprSegment parse_path_expr_segment ();
AST::QualifiedPathInExpression
- parse_qualified_path_in_expression (bool pratt_parse = false);
- AST::QualifiedPathType parse_qualified_path_type (bool pratt_parse = false);
+ parse_qualified_path_in_expression (const_TokenPtr pratt_parsed_token
+ = nullptr);
+ AST::QualifiedPathType
+ parse_qualified_path_type (const_TokenPtr pratt_parsed_token = nullptr);
AST::QualifiedPathInType parse_qualified_path_in_type ();
// Token tree or macro related
@@ -470,32 +472,32 @@ private:
parse_expr_with_block (AST::AttrVec outer_attrs);
std::unique_ptr<AST::ExprWithoutBlock>
parse_expr_without_block (AST::AttrVec outer_attrs = AST::AttrVec ());
- std::unique_ptr<AST::BlockExpr> parse_block_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfExpr> parse_if_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfLetExpr> parse_if_let_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::BlockExpr>
+ parse_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::IfExpr>
+ parse_if_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::IfLetExpr>
+ parse_if_let_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::LoopExpr>
parse_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::WhileLoopExpr>
parse_while_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::WhileLetLoopExpr>
parse_while_let_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
std::unique_ptr<AST::ForLoopExpr>
parse_for_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
- std::unique_ptr<AST::MatchExpr> parse_match_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::MatchExpr>
+ parse_match_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
AST::MatchArm parse_match_arm ();
std::vector<std::unique_ptr<AST::Pattern> >
parse_match_arm_patterns (TokenId end_token_id);
@@ -511,24 +513,24 @@ private:
AST::ClosureParam parse_closure_param ();
std::unique_ptr<AST::LiteralExpr> parse_literal_expr (AST::AttrVec outer_attrs
= AST::AttrVec ());
- std::unique_ptr<AST::ReturnExpr> parse_return_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::BreakExpr> parse_break_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::ReturnExpr>
+ parse_return_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::BreakExpr>
+ parse_break_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::ContinueExpr>
parse_continue_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::UnsafeBlockExpr>
parse_unsafe_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::ArrayExpr> parse_array_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::ArrayExpr>
+ parse_array_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::ExprWithoutBlock>
parse_grouped_or_tuple_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::StructExprField> parse_struct_expr_field ();
bool will_be_expr_with_block ();
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-07-28 22:13 [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus Mark Wielaard
@ 2021-07-29 1:25 ` The Other
2021-07-29 10:55 ` Mark Wielaard
0 siblings, 1 reply; 10+ messages in thread
From: The Other @ 2021-07-29 1:25 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gcc-rust
I think the core idea of this patch (fixing locations) is very important and useful.
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.
> On 29 Jul 2021, at 6:13 am, Mark Wielaard <mark@klomp.org> wrote:
>
> 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 token to the expression parsing
> functions (instead of just providing a pratt_parse boolean). Use this
> token 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 | 180 +++++++++++++------------------
> gcc/rust/parse/rust-parse.h | 58 +++++-----
> 2 files changed, 103 insertions(+), 135 deletions(-)
>
> diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
> index 340fea70201..6241a972ff7 100644
> --- a/gcc/rust/parse/rust-parse-impl.h
> +++ b/gcc/rust/parse/rust-parse-impl.h
> @@ -6580,7 +6580,7 @@ Parser<ManagedTokenSource>::parse_path_expr_segment ()
> template <typename ManagedTokenSource>
> AST::QualifiedPathInExpression
> Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> /* 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<ManagedTokenSource>::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_token);
> if (qual_path_type.is_error ())
> {
> // TODO: should this create a parse error?
> @@ -6661,12 +6661,13 @@ Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
> // Parses the type syntactical construction at the start of a qualified path.
> template <typename ManagedTokenSource>
> AST::QualifiedPathType
> -Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
> +Parser<ManagedTokenSource>::parse_qualified_path_type (
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> + Location locus;
> /* TODO: should this actually be error? is there anywhere where this could be
> * valid? */
> - if (!pratt_parse)
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> if (!skip_token (LEFT_ANGLE))
> @@ -6676,10 +6677,7 @@ Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
> }
> }
> else
> - {
> - // move back by 1 if pratt parsing due to skipping '<'
> - locus = lexer.peek_token ()->get_locus () - 1;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // parse type (required)
> std::unique_ptr<AST::Type> type = parse_type ();
> @@ -7305,10 +7303,10 @@ Parser<ManagedTokenSource>::parse_expr_without_block (AST::AttrVec outer_attrs)
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::BlockExpr>
> Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> if (!skip_token (LEFT_CURLY))
> @@ -7318,9 +7316,7 @@ Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
> }
> }
> else
> - {
> - locus = lexer.peek_token ()->get_locus () - 1;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> AST::AttrVec inner_attrs = parse_inner_attributes ();
>
> @@ -7611,22 +7607,17 @@ Parser<ManagedTokenSource>::parse_literal_expr (AST::AttrVec outer_attrs)
> // Parses a return expression (including any expression to return).
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::ReturnExpr>
> -Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> +Parser<ManagedTokenSource>::parse_return_expr (
> + AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> -
> skip_token (RETURN_TOK);
> }
> else
> - {
> - // minus 7 chars for 6 in return and a space
> - // or just TODO: pass in location data
> - locus = lexer.peek_token ()->get_locus () - 7;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // parse expression to return, if it exists
> ParseRestrictions restrictions;
> @@ -7644,21 +7635,16 @@ Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::BreakExpr>
> Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> -
> skip_token (BREAK);
> }
> else
> - {
> - // minus 6 chars for 5 in return and a space
> - // or just TODO: pass in location data
> - locus = lexer.peek_token ()->get_locus () - 6;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // parse label (lifetime) if it exists - create dummy first
> AST::Lifetime label = AST::Lifetime::error ();
> @@ -7681,22 +7667,17 @@ Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
> // Parses a continue expression (including any label to continue from).
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::ContinueExpr>
> -Parser<ManagedTokenSource>::parse_continue_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> +Parser<ManagedTokenSource>::parse_continue_expr (
> + AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> -
> skip_token (CONTINUE);
> }
> else
> - {
> - // minus 9 chars for 8 in return and a space
> - // or just TODO: pass in location data
> - locus = lexer.peek_token ()->get_locus () - 9;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // parse label (lifetime) if it exists - create dummy first
> AST::Lifetime label = AST::Lifetime::error ();
> @@ -7740,11 +7721,11 @@ Parser<ManagedTokenSource>::parse_loop_label ()
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::IfExpr>
> Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> // TODO: make having outer attributes an error?
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> if (!skip_token (IF))
> @@ -7754,9 +7735,7 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
> }
> }
> else
> - {
> - locus = lexer.peek_token ()->get_locus () - 1;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // detect accidental if let
> if (lexer.peek_token ()->get_id () == LET)
> @@ -7901,12 +7880,12 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
> * expressions don't support them. */
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::IfLetExpr>
> -Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> +Parser<ManagedTokenSource>::parse_if_let_expr (
> + AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
> {
> // TODO: make having outer attributes an error?
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> if (!skip_token (IF))
> @@ -7916,9 +7895,7 @@ Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
> }
> }
> else
> - {
> - locus = lexer.peek_token ()->get_locus () - 1;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // detect accidental if expr parsed as if let expr
> if (lexer.peek_token ()->get_id () != LET)
> @@ -8094,10 +8071,10 @@ template <typename ManagedTokenSource>
> std::unique_ptr<AST::LoopExpr>
> Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
> AST::LoopLabel label,
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> if (label.is_error ())
> locus = lexer.peek_token ()->get_locus ();
> @@ -8113,7 +8090,7 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
> else
> {
> if (label.is_error ())
> - locus = lexer.peek_token ()->get_locus () - 1;
> + locus = pratt_parsed_token->get_locus ();
> else
> locus = label.get_locus ();
> }
> @@ -8138,12 +8115,12 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
> * via parse_labelled_loop_expr, which would call this. */
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::WhileLoopExpr>
> -Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
> - AST::LoopLabel label,
> - bool pratt_parse)
> +Parser<ManagedTokenSource>::parse_while_loop_expr (
> + AST::AttrVec outer_attrs, AST::LoopLabel label,
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> if (label.is_error ())
> locus = lexer.peek_token ()->get_locus ();
> @@ -8159,7 +8136,7 @@ Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
> else
> {
> if (label.is_error ())
> - locus = lexer.peek_token ()->get_locus () - 1;
> + locus = pratt_parsed_token->get_locus ();
> else
> locus = label.get_locus ();
> }
> @@ -8417,21 +8394,16 @@ Parser<ManagedTokenSource>::parse_labelled_loop_expr (AST::AttrVec outer_attrs)
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::MatchExpr>
> Parser<ManagedTokenSource>::parse_match_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> -
> skip_token (MATCH_TOK);
> }
> else
> - {
> - // TODO: probably just pass in location data as param
> - // get current pos then move back 6 - 5 for match, 1 for space
> - locus = lexer.peek_token ()->get_locus () - 6;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> /* parse scrutinee expression, which is required (and HACK to prevent struct
> * expr) */
> @@ -8704,17 +8676,17 @@ Parser<ManagedTokenSource>::parse_async_block_expr (AST::AttrVec outer_attrs)
> // Parses an unsafe block expression.
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::UnsafeBlockExpr>
> -Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> +Parser<ManagedTokenSource>::parse_unsafe_block_expr (
> + AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
> {
> Location locus;
> - if (!pratt_parse)
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> skip_token (UNSAFE);
> }
> else
> - locus = lexer.peek_token ()->get_locus () - 1;
> + locus = pratt_parsed_token->get_locus ();
>
> // parse block expression (required)
> std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
> @@ -8738,19 +8710,16 @@ Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::ArrayExpr>
> Parser<ManagedTokenSource>::parse_array_expr (AST::AttrVec outer_attrs,
> - bool pratt_parse)
> + const_TokenPtr pratt_parsed_token)
> {
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> -
> skip_token (LEFT_SQUARE);
> }
> else
> - {
> - locus = lexer.peek_token ()->get_locus () - 1;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // parse optional inner attributes
> AST::AttrVec inner_attrs = parse_inner_attributes ();
> @@ -8927,20 +8896,17 @@ Parser<ManagedTokenSource>::parse_closure_param ()
> template <typename ManagedTokenSource>
> std::unique_ptr<AST::ExprWithoutBlock>
> Parser<ManagedTokenSource>::parse_grouped_or_tuple_expr (
> - AST::AttrVec outer_attrs, bool pratt_parse)
> + AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
> {
> // adjustment to allow Pratt parsing to reuse function without copy-paste
> - Location locus = Linemap::unknown_location ();
> - if (!pratt_parse)
> + Location locus;
> + if (pratt_parsed_token == nullptr)
> {
> locus = lexer.peek_token ()->get_locus ();
> -
> skip_token (LEFT_PAREN);
> }
> else
> - {
> - locus = lexer.peek_token ()->get_locus () - 1;
> - }
> + locus = pratt_parsed_token->get_locus ();
>
> // parse optional inner attributes
> AST::AttrVec inner_attrs = parse_inner_attributes ();
> @@ -12535,7 +12501,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
> // qualified path
> // HACK: add outer attrs to path
> AST::QualifiedPathInExpression path
> - = parse_qualified_path_in_expression (true);
> + = parse_qualified_path_in_expression (tok);
> path.set_outer_attrs (std::move (outer_attrs));
> return std::unique_ptr<AST::QualifiedPathInExpression> (
> new AST::QualifiedPathInExpression (std::move (path)));
> @@ -12578,7 +12544,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
> new AST::LiteralExpr ("false", AST::Literal::BOOL,
> tok->get_type_hint (), {}, tok->get_locus ()));
> case LEFT_PAREN:
> - return parse_grouped_or_tuple_expr (std::move (outer_attrs), true);
> + return parse_grouped_or_tuple_expr (std::move (outer_attrs), tok);
>
> /*case PLUS: { // unary plus operator
> // invoke parse_expr recursively with appropriate priority, etc. for
> @@ -12810,41 +12776,41 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
> return parse_range_to_inclusive_expr (tok, std::move (outer_attrs));
> case RETURN_TOK:
> // FIXME: is this really a null denotation expression?
> - return parse_return_expr (std::move (outer_attrs), true);
> + return parse_return_expr (std::move (outer_attrs), tok);
> case BREAK:
> // FIXME: is this really a null denotation expression?
> - return parse_break_expr (std::move (outer_attrs), true);
> + return parse_break_expr (std::move (outer_attrs), tok);
> case CONTINUE:
> - return parse_continue_expr (std::move (outer_attrs), true);
> + return parse_continue_expr (std::move (outer_attrs), tok);
> case LEFT_CURLY:
> // ok - this is an expression with block for once.
> - return parse_block_expr (std::move (outer_attrs), true);
> + return parse_block_expr (std::move (outer_attrs), tok);
> case IF:
> // if or if let, so more lookahead to find out
> if (lexer.peek_token (1)->get_id () == LET)
> {
> // if let expr
> - return parse_if_let_expr (std::move (outer_attrs), true);
> + return parse_if_let_expr (std::move (outer_attrs), tok);
> }
> else
> {
> // if expr
> - return parse_if_expr (std::move (outer_attrs), true);
> + return parse_if_expr (std::move (outer_attrs), tok);
> }
> case LOOP:
> return parse_loop_expr (std::move (outer_attrs), AST::LoopLabel::error (),
> - true);
> + tok);
> case WHILE:
> return parse_while_loop_expr (std::move (outer_attrs),
> - AST::LoopLabel::error (), true);
> + AST::LoopLabel::error (), tok);
> case MATCH_TOK:
> // also an expression with block
> - return parse_match_expr (std::move (outer_attrs), true);
> + return parse_match_expr (std::move (outer_attrs), tok);
> case LEFT_SQUARE:
> // array definition expr (not indexing)
> - return parse_array_expr (std::move (outer_attrs), true);
> + return parse_array_expr (std::move (outer_attrs), tok);
> case UNSAFE:
> - return parse_unsafe_block_expr (std::move (outer_attrs), true);
> + return parse_unsafe_block_expr (std::move (outer_attrs), tok);
> default:
> if (!restrictions.expr_can_be_null)
> add_error (Error (tok->get_locus (),
> diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
> index 1c7bd781b3f..78608839265 100644
> --- a/gcc/rust/parse/rust-parse.h
> +++ b/gcc/rust/parse/rust-parse.h
> @@ -121,8 +121,10 @@ private:
> AST::PathInExpression parse_path_in_expression ();
> AST::PathExprSegment parse_path_expr_segment ();
> AST::QualifiedPathInExpression
> - parse_qualified_path_in_expression (bool pratt_parse = false);
> - AST::QualifiedPathType parse_qualified_path_type (bool pratt_parse = false);
> + parse_qualified_path_in_expression (const_TokenPtr pratt_parsed_token
> + = nullptr);
> + AST::QualifiedPathType
> + parse_qualified_path_type (const_TokenPtr pratt_parsed_token = nullptr);
> AST::QualifiedPathInType parse_qualified_path_in_type ();
>
> // Token tree or macro related
> @@ -470,32 +472,32 @@ private:
> parse_expr_with_block (AST::AttrVec outer_attrs);
> std::unique_ptr<AST::ExprWithoutBlock>
> parse_expr_without_block (AST::AttrVec outer_attrs = AST::AttrVec ());
> - std::unique_ptr<AST::BlockExpr> parse_block_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> - std::unique_ptr<AST::IfExpr> parse_if_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> - std::unique_ptr<AST::IfLetExpr> parse_if_let_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> + std::unique_ptr<AST::BlockExpr>
> + parse_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> + std::unique_ptr<AST::IfExpr>
> + parse_if_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> + std::unique_ptr<AST::IfLetExpr>
> + parse_if_let_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::LoopExpr>
> parse_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> AST::LoopLabel label = AST::LoopLabel::error (),
> - bool pratt_parse = false);
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::WhileLoopExpr>
> parse_while_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> AST::LoopLabel label = AST::LoopLabel::error (),
> - bool pratt_parse = false);
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::WhileLetLoopExpr>
> parse_while_let_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> AST::LoopLabel label = AST::LoopLabel::error ());
> std::unique_ptr<AST::ForLoopExpr>
> parse_for_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> AST::LoopLabel label = AST::LoopLabel::error ());
> - std::unique_ptr<AST::MatchExpr> parse_match_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> + std::unique_ptr<AST::MatchExpr>
> + parse_match_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> AST::MatchArm parse_match_arm ();
> std::vector<std::unique_ptr<AST::Pattern> >
> parse_match_arm_patterns (TokenId end_token_id);
> @@ -511,24 +513,24 @@ private:
> AST::ClosureParam parse_closure_param ();
> std::unique_ptr<AST::LiteralExpr> parse_literal_expr (AST::AttrVec outer_attrs
> = AST::AttrVec ());
> - std::unique_ptr<AST::ReturnExpr> parse_return_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> - std::unique_ptr<AST::BreakExpr> parse_break_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> + std::unique_ptr<AST::ReturnExpr>
> + parse_return_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> + std::unique_ptr<AST::BreakExpr>
> + parse_break_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::ContinueExpr>
> parse_continue_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> - bool pratt_parse = false);
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::UnsafeBlockExpr>
> parse_unsafe_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> - bool pratt_parse = false);
> - std::unique_ptr<AST::ArrayExpr> parse_array_expr (AST::AttrVec outer_attrs
> - = AST::AttrVec (),
> - bool pratt_parse = false);
> + const_TokenPtr pratt_parsed_token = nullptr);
> + std::unique_ptr<AST::ArrayExpr>
> + parse_array_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::ExprWithoutBlock>
> parse_grouped_or_tuple_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
> - bool pratt_parse = false);
> + const_TokenPtr pratt_parsed_token = nullptr);
> std::unique_ptr<AST::StructExprField> parse_struct_expr_field ();
> bool will_be_expr_with_block ();
>
> --
> 2.32.0
>
> --
> Gcc-rust mailing list
> Gcc-rust@gcc.gnu.org
> https://gcc.gnu.org/mailman/listinfo/gcc-rust
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-07-29 1:25 ` The Other
@ 2021-07-29 10:55 ` Mark Wielaard
2021-07-29 15:18 ` Thomas Schwinge
0 siblings, 1 reply; 10+ messages in thread
From: Mark Wielaard @ 2021-07-29 10:55 UTC (permalink / raw)
To: The Other; +Cc: gcc-rust
Hi Joel,
On Thu, 2021-07-29 at 09:25 +0800, The Other via Gcc-rust wrote:
> I think the core idea of this patch (fixing locations) is very
> important and useful.
>
> 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. My idea was that the code might want to double
check/assert it was the correct token (as it does in the case the
parser function is called directly and it has to consume the first
token itself).
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.
Thanks,
Mark
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-07-29 10:55 ` Mark Wielaard
@ 2021-07-29 15:18 ` Thomas Schwinge
2021-07-30 1:03 ` Mark Wielaard
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Schwinge @ 2021-07-29 15:18 UTC (permalink / raw)
To: Mark Wielaard, simplytheother; +Cc: gcc-rust
Hi!
On 2021-07-29T12:55:38+0200, Mark Wielaard <mark@klomp.org> wrote:
> On Thu, 2021-07-29 at 09:25 +0800, The Other via Gcc-rust wrote:
>> I think the core idea of this patch (fixing locations) is very
>> important and useful.
Agreed.
>> 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.)
> My idea was that the code might want to double
> check/assert it was the correct token (as it does in the case the
> parser function is called directly and it has to consume the first
> token itself).
That also makes sense to me. (Defensive programming; verify input data
to be what you expect to see.)
Grüße
Thomas
> 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.
>
> Thanks,
>
> Mark
> --
> Gcc-rust mailing list
> Gcc-rust@gcc.gnu.org
> https://gcc.gnu.org/mailman/listinfo/gcc-rust
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-07-29 15:18 ` Thomas Schwinge
@ 2021-07-30 1:03 ` Mark Wielaard
2021-08-05 19:37 ` Mark Wielaard
0 siblings, 1 reply; 10+ messages in thread
From: Mark Wielaard @ 2021-07-30 1:03 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: simplytheother, gcc-rust
[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]
Hi,
On Thu, Jul 29, 2021 at 05:18:50PM +0200, Thomas Schwinge wrote:
> On 2021-07-29T12:55:38+0200, Mark Wielaard <mark@klomp.org> 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
[-- Attachment #2: 0001-Pass-pratt-parsed-location-to-expr-parser-functions-.patch --]
[-- Type: text/x-diff, Size: 22003 bytes --]
From 2c51713eec25b38ab2274537103a391d89894dba Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
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<ManagedTokenSource>::parse_path_expr_segment ()
template <typename ManagedTokenSource>
AST::QualifiedPathInExpression
Parser<ManagedTokenSource>::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<ManagedTokenSource>::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<ManagedTokenSource>::parse_qualified_path_in_expression (
// Parses the type syntactical construction at the start of a qualified path.
template <typename ManagedTokenSource>
AST::QualifiedPathType
-Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
+Parser<ManagedTokenSource>::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<ManagedTokenSource>::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<AST::Type> type = parse_type ();
@@ -7305,10 +7301,10 @@ Parser<ManagedTokenSource>::parse_expr_without_block (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::BlockExpr>
Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_CURLY))
@@ -7317,10 +7313,6 @@ Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
return nullptr;
}
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -7612,21 +7604,14 @@ Parser<ManagedTokenSource>::parse_literal_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::ReturnExpr>
Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (RETURN_TOK);
}
- else
- {
- // minus 7 chars for 6 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 7;
- }
// parse expression to return, if it exists
ParseRestrictions restrictions;
@@ -7644,21 +7629,14 @@ Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::BreakExpr>
Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (BREAK);
}
- else
- {
- // minus 6 chars for 5 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 6;
- }
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7682,21 +7660,14 @@ Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::ContinueExpr>
Parser<ManagedTokenSource>::parse_continue_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (CONTINUE);
}
- else
- {
- // minus 9 chars for 8 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 9;
- }
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7740,11 +7711,11 @@ Parser<ManagedTokenSource>::parse_loop_label ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfExpr>
Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7753,10 +7724,6 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
return nullptr;
}
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// detect accidental if let
if (lexer.peek_token ()->get_id () == LET)
@@ -7902,11 +7869,11 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfLetExpr>
Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7915,10 +7882,6 @@ Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
return nullptr;
}
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// detect accidental if expr parsed as if let expr
if (lexer.peek_token ()->get_id () != LET)
@@ -8094,10 +8057,10 @@ template <typename ManagedTokenSource>
std::unique_ptr<AST::LoopExpr>
Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
AST::LoopLabel label,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8112,9 +8075,7 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
}
else
{
- if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
- else
+ if (!label.is_error ())
locus = label.get_locus ();
}
@@ -8140,10 +8101,10 @@ template <typename ManagedTokenSource>
std::unique_ptr<AST::WhileLoopExpr>
Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
AST::LoopLabel label,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8158,9 +8119,7 @@ Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
}
else
{
- if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
- else
+ if (!label.is_error ())
locus = label.get_locus ();
}
@@ -8417,21 +8376,14 @@ Parser<ManagedTokenSource>::parse_labelled_loop_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::MatchExpr>
Parser<ManagedTokenSource>::parse_match_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (MATCH_TOK);
}
- else
- {
- // TODO: probably just pass in location data as param
- // get current pos then move back 6 - 5 for match, 1 for space
- locus = lexer.peek_token ()->get_locus () - 6;
- }
/* parse scrutinee expression, which is required (and HACK to prevent struct
* expr) */
@@ -8705,16 +8657,14 @@ Parser<ManagedTokenSource>::parse_async_block_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::UnsafeBlockExpr>
Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus;
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
skip_token (UNSAFE);
}
- else
- locus = lexer.peek_token ()->get_locus () - 1;
// parse block expression (required)
std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
@@ -8738,19 +8688,14 @@ Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::ArrayExpr>
Parser<ManagedTokenSource>::parse_array_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_SQUARE);
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -8927,20 +8872,15 @@ Parser<ManagedTokenSource>::parse_closure_param ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::ExprWithoutBlock>
Parser<ManagedTokenSource>::parse_grouped_or_tuple_expr (
- AST::AttrVec outer_attrs, bool pratt_parse)
+ AST::AttrVec outer_attrs, Location pratt_parsed_loc)
{
// adjustment to allow Pratt parsing to reuse function without copy-paste
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_PAREN);
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -12535,7 +12475,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
// qualified path
// HACK: add outer attrs to path
AST::QualifiedPathInExpression path
- = parse_qualified_path_in_expression (true);
+ = parse_qualified_path_in_expression (tok->get_locus ());
path.set_outer_attrs (std::move (outer_attrs));
return std::unique_ptr<AST::QualifiedPathInExpression> (
new AST::QualifiedPathInExpression (std::move (path)));
@@ -12578,7 +12518,8 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
new AST::LiteralExpr ("false", AST::Literal::BOOL,
tok->get_type_hint (), {}, tok->get_locus ()));
case LEFT_PAREN:
- return parse_grouped_or_tuple_expr (std::move (outer_attrs), true);
+ return parse_grouped_or_tuple_expr (std::move (outer_attrs),
+ tok->get_locus ());
/*case PLUS: { // unary plus operator
// invoke parse_expr recursively with appropriate priority, etc. for
@@ -12810,41 +12751,43 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
return parse_range_to_inclusive_expr (tok, std::move (outer_attrs));
case RETURN_TOK:
// FIXME: is this really a null denotation expression?
- return parse_return_expr (std::move (outer_attrs), true);
+ return parse_return_expr (std::move (outer_attrs), tok->get_locus ());
case BREAK:
// FIXME: is this really a null denotation expression?
- return parse_break_expr (std::move (outer_attrs), true);
+ return parse_break_expr (std::move (outer_attrs), tok->get_locus ());
case CONTINUE:
- return parse_continue_expr (std::move (outer_attrs), true);
+ return parse_continue_expr (std::move (outer_attrs), tok->get_locus ());
case LEFT_CURLY:
// ok - this is an expression with block for once.
- return parse_block_expr (std::move (outer_attrs), true);
+ return parse_block_expr (std::move (outer_attrs), tok->get_locus ());
case IF:
// if or if let, so more lookahead to find out
if (lexer.peek_token (1)->get_id () == LET)
{
// if let expr
- return parse_if_let_expr (std::move (outer_attrs), true);
+ return parse_if_let_expr (std::move (outer_attrs), tok->get_locus ());
}
else
{
// if expr
- return parse_if_expr (std::move (outer_attrs), true);
+ return parse_if_expr (std::move (outer_attrs), tok->get_locus ());
}
case LOOP:
return parse_loop_expr (std::move (outer_attrs), AST::LoopLabel::error (),
- true);
+ tok->get_locus ());
case WHILE:
return parse_while_loop_expr (std::move (outer_attrs),
- AST::LoopLabel::error (), true);
+ AST::LoopLabel::error (),
+ tok->get_locus ());
case MATCH_TOK:
// also an expression with block
- return parse_match_expr (std::move (outer_attrs), true);
+ return parse_match_expr (std::move (outer_attrs), tok->get_locus ());
case LEFT_SQUARE:
// array definition expr (not indexing)
- return parse_array_expr (std::move (outer_attrs), true);
+ return parse_array_expr (std::move (outer_attrs), tok->get_locus ());
case UNSAFE:
- return parse_unsafe_block_expr (std::move (outer_attrs), true);
+ return parse_unsafe_block_expr (std::move (outer_attrs),
+ tok->get_locus ());
default:
if (!restrictions.expr_can_be_null)
add_error (Error (tok->get_locus (),
diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
index 1c7bd781b3f..ec68abf8995 100644
--- a/gcc/rust/parse/rust-parse.h
+++ b/gcc/rust/parse/rust-parse.h
@@ -121,8 +121,11 @@ private:
AST::PathInExpression parse_path_in_expression ();
AST::PathExprSegment parse_path_expr_segment ();
AST::QualifiedPathInExpression
- parse_qualified_path_in_expression (bool pratt_parse = false);
- AST::QualifiedPathType parse_qualified_path_type (bool pratt_parse = false);
+ parse_qualified_path_in_expression (Location pratt_parsed_loc
+ = Linemap::unknown_location ());
+ AST::QualifiedPathType
+ parse_qualified_path_type (Location pratt_parsed_loc
+ = Linemap::unknown_location ());
AST::QualifiedPathInType parse_qualified_path_in_type ();
// Token tree or macro related
@@ -470,32 +473,33 @@ private:
parse_expr_with_block (AST::AttrVec outer_attrs);
std::unique_ptr<AST::ExprWithoutBlock>
parse_expr_without_block (AST::AttrVec outer_attrs = AST::AttrVec ());
- std::unique_ptr<AST::BlockExpr> parse_block_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfExpr> parse_if_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfLetExpr> parse_if_let_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::BlockExpr>
+ parse_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
+ std::unique_ptr<AST::IfExpr>
+ parse_if_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
+ std::unique_ptr<AST::IfLetExpr>
+ parse_if_let_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::LoopExpr>
parse_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::WhileLoopExpr>
parse_while_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
std::unique_ptr<AST::WhileLetLoopExpr>
parse_while_let_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
std::unique_ptr<AST::ForLoopExpr>
parse_for_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
- std::unique_ptr<AST::MatchExpr> parse_match_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::MatchExpr>
+ parse_match_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
AST::MatchArm parse_match_arm ();
std::vector<std::unique_ptr<AST::Pattern> >
parse_match_arm_patterns (TokenId end_token_id);
@@ -511,24 +515,27 @@ private:
AST::ClosureParam parse_closure_param ();
std::unique_ptr<AST::LiteralExpr> parse_literal_expr (AST::AttrVec outer_attrs
= AST::AttrVec ());
- std::unique_ptr<AST::ReturnExpr> parse_return_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::BreakExpr> parse_break_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::ReturnExpr>
+ parse_return_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
+ std::unique_ptr<AST::BreakExpr>
+ parse_break_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::ContinueExpr>
parse_continue_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
std::unique_ptr<AST::UnsafeBlockExpr>
parse_unsafe_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::ArrayExpr> parse_array_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
+ std::unique_ptr<AST::ArrayExpr>
+ parse_array_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::ExprWithoutBlock>
parse_grouped_or_tuple_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
std::unique_ptr<AST::StructExprField> parse_struct_expr_field ();
bool will_be_expr_with_block ();
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-07-30 1:03 ` Mark Wielaard
@ 2021-08-05 19:37 ` Mark Wielaard
2021-08-06 15:23 ` Philip Herron
0 siblings, 1 reply; 10+ messages in thread
From: Mark Wielaard @ 2021-08-05 19:37 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: gcc-rust
Hi,
On Fri, Jul 30, 2021 at 03:03:13AM +0200, Mark Wielaard wrote:
> 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.
Any feedback on these patches? I really like to get the locations
correct for these pratt parsed expressions. If both variants are
problematic please let me know what I can do to improve them.
Thanks,
Mark
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-08-05 19:37 ` Mark Wielaard
@ 2021-08-06 15:23 ` Philip Herron
2021-08-07 1:01 ` Mark Wielaard
0 siblings, 1 reply; 10+ messages in thread
From: Philip Herron @ 2021-08-06 15:23 UTC (permalink / raw)
To: gcc-rust
[-- Attachment #1.1.1: Type: text/plain, Size: 1477 bytes --]
On 05/08/2021 20:37, Mark Wielaard wrote:
> Hi,
>
> On Fri, Jul 30, 2021 at 03:03:13AM +0200, Mark Wielaard wrote:
>> 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.
> Any feedback on these patches? I really like to get the locations
> correct for these pratt parsed expressions. If both variants are
> problematic please let me know what I can do to improve them.
>
> Thanks,
>
> Mark
>
Hi Mark,
Sorry for lack of reply on this. After looking into this and also
getting Arthur Cohen to review, I think:
* Token passing: If we end up using the token for more than the
location this might be useful but at present I can't think of a
use-case for the token. If we do need it maybe we could look into
passing it by reference.
* Location passing: I think this is the best solution and is directly
associated with the issue you have fixed. The function says that you
can call me but tell me the location as well so its up to the caller
to handle this. We should add a comment to the function prototypes
to say what the location is but that's it.
What do you think?
Thanks
--Phil
[-- Attachment #1.1.2: Type: text/html, Size: 2340 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-08-06 15:23 ` Philip Herron
@ 2021-08-07 1:01 ` Mark Wielaard
2021-08-08 11:53 ` Philip Herron
2021-08-09 14:24 ` Mark Wielaard
0 siblings, 2 replies; 10+ messages in thread
From: Mark Wielaard @ 2021-08-07 1:01 UTC (permalink / raw)
To: Philip Herron; +Cc: gcc-rust
[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]
On Fri, Aug 06, 2021 at 04:23:27PM +0100, Philip Herron wrote:
> On 05/08/2021 20:37, Mark Wielaard wrote:
> > Hi,
> >
> > On Fri, Jul 30, 2021 at 03:03:13AM +0200, Mark Wielaard wrote:
> >> 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.
> > Any feedback on these patches? I really like to get the locations
> > correct for these pratt parsed expressions. If both variants are
> > problematic please let me know what I can do to improve them.
> >
> > Thanks,
> >
> > Mark
> >
> Hi Mark,
>
> Sorry for lack of reply on this. After looking into this and also
> getting Arthur Cohen to review, I think:
>
> * Token passing: If we end up using the token for more than the
> location this might be useful but at present I can't think of a
> use-case for the token. If we do need it maybe we could look into
> passing it by reference.
> * Location passing: I think this is the best solution and is directly
> associated with the issue you have fixed. The function says that you
> can call me but tell me the location as well so its up to the caller
> to handle this. We should add a comment to the function prototypes
> to say what the location is but that's it.
>
> What do you think?
OK, lets go with the location passing. I added a comment to the
function prototypes in rust-parse.h about the passed in
pratt_parsed_token.
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-token
Cheers,
Mark
[-- Attachment #2: 0001-Pass-pratt-parsed-token-to-expr-parser-functions-to-.patch --]
[-- Type: text/x-diff, Size: 22762 bytes --]
From 1c139cbaa71c2928ca747fa26dc776699937280a Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Thu, 29 Jul 2021 00:00:55 +0200
Subject: [PATCH] Pass pratt parsed token 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 token to the expression parsing
functions (instead of just providing a pratt_parse boolean). Use this
token 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 | 180 +++++++++++++------------------
gcc/rust/parse/rust-parse.h | 64 ++++++-----
2 files changed, 109 insertions(+), 135 deletions(-)
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index ccfff841784..1e951e4bd48 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -6586,7 +6586,7 @@ Parser<ManagedTokenSource>::parse_path_expr_segment ()
template <typename ManagedTokenSource>
AST::QualifiedPathInExpression
Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
/* Note: the Rust grammar is defined in such a way that it is impossible to
* determine whether a prospective qualified path is a
@@ -6601,7 +6601,7 @@ Parser<ManagedTokenSource>::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_token);
if (qual_path_type.is_error ())
{
// TODO: should this create a parse error?
@@ -6667,12 +6667,13 @@ Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
// Parses the type syntactical construction at the start of a qualified path.
template <typename ManagedTokenSource>
AST::QualifiedPathType
-Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
+Parser<ManagedTokenSource>::parse_qualified_path_type (
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
+ Location locus;
/* TODO: should this actually be error? is there anywhere where this could be
* valid? */
- if (!pratt_parse)
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_ANGLE))
@@ -6682,10 +6683,7 @@ Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
}
}
else
- {
- // move back by 1 if pratt parsing due to skipping '<'
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse type (required)
std::unique_ptr<AST::Type> type = parse_type ();
@@ -7311,10 +7309,10 @@ Parser<ManagedTokenSource>::parse_expr_without_block (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::BlockExpr>
Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_CURLY))
@@ -7324,9 +7322,7 @@ Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
}
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -7617,22 +7613,17 @@ Parser<ManagedTokenSource>::parse_literal_expr (AST::AttrVec outer_attrs)
// Parses a return expression (including any expression to return).
template <typename ManagedTokenSource>
std::unique_ptr<AST::ReturnExpr>
-Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_return_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (RETURN_TOK);
}
else
- {
- // minus 7 chars for 6 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 7;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse expression to return, if it exists
ParseRestrictions restrictions;
@@ -7650,21 +7641,16 @@ Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::BreakExpr>
Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (BREAK);
}
else
- {
- // minus 6 chars for 5 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 6;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7687,22 +7673,17 @@ Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
// Parses a continue expression (including any label to continue from).
template <typename ManagedTokenSource>
std::unique_ptr<AST::ContinueExpr>
-Parser<ManagedTokenSource>::parse_continue_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_continue_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (CONTINUE);
}
else
- {
- // minus 9 chars for 8 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 9;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7746,11 +7727,11 @@ Parser<ManagedTokenSource>::parse_loop_label ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfExpr>
Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7760,9 +7741,7 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
}
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// detect accidental if let
if (lexer.peek_token ()->get_id () == LET)
@@ -7907,12 +7886,12 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
* expressions don't support them. */
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfLetExpr>
-Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_if_let_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7922,9 +7901,7 @@ Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
}
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// detect accidental if expr parsed as if let expr
if (lexer.peek_token ()->get_id () != LET)
@@ -8100,10 +8077,10 @@ template <typename ManagedTokenSource>
std::unique_ptr<AST::LoopExpr>
Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
AST::LoopLabel label,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8119,7 +8096,7 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
else
{
if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
+ locus = pratt_parsed_token->get_locus ();
else
locus = label.get_locus ();
}
@@ -8144,12 +8121,12 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
* via parse_labelled_loop_expr, which would call this. */
template <typename ManagedTokenSource>
std::unique_ptr<AST::WhileLoopExpr>
-Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
- AST::LoopLabel label,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_while_loop_expr (
+ AST::AttrVec outer_attrs, AST::LoopLabel label,
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8165,7 +8142,7 @@ Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
else
{
if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
+ locus = pratt_parsed_token->get_locus ();
else
locus = label.get_locus ();
}
@@ -8423,21 +8400,16 @@ Parser<ManagedTokenSource>::parse_labelled_loop_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::MatchExpr>
Parser<ManagedTokenSource>::parse_match_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (MATCH_TOK);
}
else
- {
- // TODO: probably just pass in location data as param
- // get current pos then move back 6 - 5 for match, 1 for space
- locus = lexer.peek_token ()->get_locus () - 6;
- }
+ locus = pratt_parsed_token->get_locus ();
/* parse scrutinee expression, which is required (and HACK to prevent struct
* expr) */
@@ -8710,17 +8682,17 @@ Parser<ManagedTokenSource>::parse_async_block_expr (AST::AttrVec outer_attrs)
// Parses an unsafe block expression.
template <typename ManagedTokenSource>
std::unique_ptr<AST::UnsafeBlockExpr>
-Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+Parser<ManagedTokenSource>::parse_unsafe_block_expr (
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
Location locus;
- if (!pratt_parse)
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
skip_token (UNSAFE);
}
else
- locus = lexer.peek_token ()->get_locus () - 1;
+ locus = pratt_parsed_token->get_locus ();
// parse block expression (required)
std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
@@ -8744,19 +8716,16 @@ Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::ArrayExpr>
Parser<ManagedTokenSource>::parse_array_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ const_TokenPtr pratt_parsed_token)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_SQUARE);
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -8933,20 +8902,17 @@ Parser<ManagedTokenSource>::parse_closure_param ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::ExprWithoutBlock>
Parser<ManagedTokenSource>::parse_grouped_or_tuple_expr (
- AST::AttrVec outer_attrs, bool pratt_parse)
+ AST::AttrVec outer_attrs, const_TokenPtr pratt_parsed_token)
{
// adjustment to allow Pratt parsing to reuse function without copy-paste
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus;
+ if (pratt_parsed_token == nullptr)
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_PAREN);
}
else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
+ locus = pratt_parsed_token->get_locus ();
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -12541,7 +12507,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
// qualified path
// HACK: add outer attrs to path
AST::QualifiedPathInExpression path
- = parse_qualified_path_in_expression (true);
+ = parse_qualified_path_in_expression (tok);
path.set_outer_attrs (std::move (outer_attrs));
return std::unique_ptr<AST::QualifiedPathInExpression> (
new AST::QualifiedPathInExpression (std::move (path)));
@@ -12584,7 +12550,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
new AST::LiteralExpr ("false", AST::Literal::BOOL,
tok->get_type_hint (), {}, tok->get_locus ()));
case LEFT_PAREN:
- return parse_grouped_or_tuple_expr (std::move (outer_attrs), true);
+ return parse_grouped_or_tuple_expr (std::move (outer_attrs), tok);
/*case PLUS: { // unary plus operator
// invoke parse_expr recursively with appropriate priority, etc. for
@@ -12816,41 +12782,41 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
return parse_range_to_inclusive_expr (tok, std::move (outer_attrs));
case RETURN_TOK:
// FIXME: is this really a null denotation expression?
- return parse_return_expr (std::move (outer_attrs), true);
+ return parse_return_expr (std::move (outer_attrs), tok);
case BREAK:
// FIXME: is this really a null denotation expression?
- return parse_break_expr (std::move (outer_attrs), true);
+ return parse_break_expr (std::move (outer_attrs), tok);
case CONTINUE:
- return parse_continue_expr (std::move (outer_attrs), true);
+ return parse_continue_expr (std::move (outer_attrs), tok);
case LEFT_CURLY:
// ok - this is an expression with block for once.
- return parse_block_expr (std::move (outer_attrs), true);
+ return parse_block_expr (std::move (outer_attrs), tok);
case IF:
// if or if let, so more lookahead to find out
if (lexer.peek_token (1)->get_id () == LET)
{
// if let expr
- return parse_if_let_expr (std::move (outer_attrs), true);
+ return parse_if_let_expr (std::move (outer_attrs), tok);
}
else
{
// if expr
- return parse_if_expr (std::move (outer_attrs), true);
+ return parse_if_expr (std::move (outer_attrs), tok);
}
case LOOP:
return parse_loop_expr (std::move (outer_attrs), AST::LoopLabel::error (),
- true);
+ tok);
case WHILE:
return parse_while_loop_expr (std::move (outer_attrs),
- AST::LoopLabel::error (), true);
+ AST::LoopLabel::error (), tok);
case MATCH_TOK:
// also an expression with block
- return parse_match_expr (std::move (outer_attrs), true);
+ return parse_match_expr (std::move (outer_attrs), tok);
case LEFT_SQUARE:
// array definition expr (not indexing)
- return parse_array_expr (std::move (outer_attrs), true);
+ return parse_array_expr (std::move (outer_attrs), tok);
case UNSAFE:
- return parse_unsafe_block_expr (std::move (outer_attrs), true);
+ return parse_unsafe_block_expr (std::move (outer_attrs), tok);
default:
if (!restrictions.expr_can_be_null)
add_error (Error (tok->get_locus (),
diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
index 3920893faf6..10c99aa8898 100644
--- a/gcc/rust/parse/rust-parse.h
+++ b/gcc/rust/parse/rust-parse.h
@@ -120,9 +120,13 @@ private:
AST::TypePathFunction parse_type_path_function ();
AST::PathInExpression parse_path_in_expression ();
AST::PathExprSegment parse_path_expr_segment ();
+ // When given a pratt_parsed_token, use it as the first token parsed
+ // in the expression.
AST::QualifiedPathInExpression
- parse_qualified_path_in_expression (bool pratt_parse = false);
- AST::QualifiedPathType parse_qualified_path_type (bool pratt_parse = false);
+ parse_qualified_path_in_expression (const_TokenPtr pratt_parsed_token
+ = nullptr);
+ AST::QualifiedPathType
+ parse_qualified_path_type (const_TokenPtr pratt_parsed_token = nullptr);
AST::QualifiedPathInType parse_qualified_path_in_type ();
// Token tree or macro related
@@ -469,32 +473,34 @@ private:
parse_expr_with_block (AST::AttrVec outer_attrs);
std::unique_ptr<AST::ExprWithoutBlock>
parse_expr_without_block (AST::AttrVec outer_attrs = AST::AttrVec ());
- std::unique_ptr<AST::BlockExpr> parse_block_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfExpr> parse_if_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfLetExpr> parse_if_let_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ // When given a pratt_parsed_token, use it as the first token parsed
+ // in the expression.
+ std::unique_ptr<AST::BlockExpr>
+ parse_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::IfExpr>
+ parse_if_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::IfLetExpr>
+ parse_if_let_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::LoopExpr>
parse_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::WhileLoopExpr>
parse_while_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::WhileLetLoopExpr>
parse_while_let_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
std::unique_ptr<AST::ForLoopExpr>
parse_for_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
- std::unique_ptr<AST::MatchExpr> parse_match_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::MatchExpr>
+ parse_match_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
AST::MatchArm parse_match_arm ();
std::vector<std::unique_ptr<AST::Pattern> >
parse_match_arm_patterns (TokenId end_token_id);
@@ -510,24 +516,26 @@ private:
AST::ClosureParam parse_closure_param ();
std::unique_ptr<AST::LiteralExpr> parse_literal_expr (AST::AttrVec outer_attrs
= AST::AttrVec ());
- std::unique_ptr<AST::ReturnExpr> parse_return_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::BreakExpr> parse_break_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ // When given a pratt_parsed_token, use it as the first token parsed
+ // in the expression.
+ std::unique_ptr<AST::ReturnExpr>
+ parse_return_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::BreakExpr>
+ parse_break_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::ContinueExpr>
parse_continue_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::UnsafeBlockExpr>
parse_unsafe_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::ArrayExpr> parse_array_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
+ std::unique_ptr<AST::ArrayExpr>
+ parse_array_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::ExprWithoutBlock>
parse_grouped_or_tuple_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ const_TokenPtr pratt_parsed_token = nullptr);
std::unique_ptr<AST::StructExprField> parse_struct_expr_field ();
bool will_be_expr_with_block ();
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-08-07 1:01 ` Mark Wielaard
@ 2021-08-08 11:53 ` Philip Herron
2021-08-09 14:24 ` Mark Wielaard
1 sibling, 0 replies; 10+ messages in thread
From: Philip Herron @ 2021-08-08 11:53 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gcc-rust
[-- Attachment #1.1: Type: text/plain, Size: 1961 bytes --]
On 07/08/2021 02:01, Mark Wielaard wrote:
> On Fri, Aug 06, 2021 at 04:23:27PM +0100, Philip Herron wrote:
>> On 05/08/2021 20:37, Mark Wielaard wrote:
>>> Hi,
>>>
>>> On Fri, Jul 30, 2021 at 03:03:13AM +0200, Mark Wielaard wrote:
>>>> 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.
>>> Any feedback on these patches? I really like to get the locations
>>> correct for these pratt parsed expressions. If both variants are
>>> problematic please let me know what I can do to improve them.
>>>
>>> Thanks,
>>>
>>> Mark
>>>
>> Hi Mark,
>>
>> Sorry for lack of reply on this. After looking into this and also
>> getting Arthur Cohen to review, I think:
>>
>> * Token passing: If we end up using the token for more than the
>> location this might be useful but at present I can't think of a
>> use-case for the token. If we do need it maybe we could look into
>> passing it by reference.
>> * Location passing: I think this is the best solution and is directly
>> associated with the issue you have fixed. The function says that you
>> can call me but tell me the location as well so its up to the caller
>> to handle this. We should add a comment to the function prototypes
>> to say what the location is but that's it.
>>
>> What do you think?
> OK, lets go with the location passing. I added a comment to the
> function prototypes in rust-parse.h about the passed in
> pratt_parsed_token.
>
> https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-token
>
> Cheers,
>
> Mark
Thanks Mark, I will merge this one.
--Phil
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
2021-08-07 1:01 ` Mark Wielaard
2021-08-08 11:53 ` Philip Herron
@ 2021-08-09 14:24 ` Mark Wielaard
1 sibling, 0 replies; 10+ messages in thread
From: Mark Wielaard @ 2021-08-09 14:24 UTC (permalink / raw)
To: Philip Herron; +Cc: gcc-rust
[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]
Hi,
On Sat, Aug 07, 2021 at 03:01:39AM +0200, Mark Wielaard wrote:
> > Sorry for lack of reply on this. After looking into this and also
> > getting Arthur Cohen to review, I think:
> >
> > * Token passing: If we end up using the token for more than the
> > location this might be useful but at present I can't think of a
> > use-case for the token. If we do need it maybe we could look into
> > passing it by reference.
> > * Location passing: I think this is the best solution and is directly
> > associated with the issue you have fixed. The function says that you
> > can call me but tell me the location as well so its up to the caller
> > to handle this. We should add a comment to the function prototypes
> > to say what the location is but that's it.
> >
> > What do you think?
>
> OK, lets go with the location passing. I added a comment to the
> function prototypes in rust-parse.h about the passed in
> pratt_parsed_token.
>
> https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-token
Looks like I said I'll do B, then did A anyway... duh. Sorry. Here is
the patch as I said I would do it, also on this branch, rebased
against upstream:
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-loc
Cheers,
Mark
[-- Attachment #2: 0001-Pass-pratt-parsed-location-to-expr-parser-functions-.patch --]
[-- Type: text/x-diff, Size: 22586 bytes --]
From 5b0bbf4509776b5ce3e5f82b8365f62110f91641 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
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 (and as
indicator to not try to parse the first token 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 | 72 ++++++++-----
2 files changed, 104 insertions(+), 145 deletions(-)
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index ccfff841784..c94c637e45c 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -6586,7 +6586,7 @@ Parser<ManagedTokenSource>::parse_path_expr_segment ()
template <typename ManagedTokenSource>
AST::QualifiedPathInExpression
Parser<ManagedTokenSource>::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
@@ -6601,7 +6601,7 @@ Parser<ManagedTokenSource>::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?
@@ -6667,12 +6667,13 @@ Parser<ManagedTokenSource>::parse_qualified_path_in_expression (
// Parses the type syntactical construction at the start of a qualified path.
template <typename ManagedTokenSource>
AST::QualifiedPathType
-Parser<ManagedTokenSource>::parse_qualified_path_type (bool pratt_parse)
+Parser<ManagedTokenSource>::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))
@@ -6681,11 +6682,6 @@ Parser<ManagedTokenSource>::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<AST::Type> type = parse_type ();
@@ -7311,10 +7307,10 @@ Parser<ManagedTokenSource>::parse_expr_without_block (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::BlockExpr>
Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (LEFT_CURLY))
@@ -7323,10 +7319,6 @@ Parser<ManagedTokenSource>::parse_block_expr (AST::AttrVec outer_attrs,
return nullptr;
}
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -7618,21 +7610,14 @@ Parser<ManagedTokenSource>::parse_literal_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::ReturnExpr>
Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (RETURN_TOK);
}
- else
- {
- // minus 7 chars for 6 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 7;
- }
// parse expression to return, if it exists
ParseRestrictions restrictions;
@@ -7650,21 +7635,14 @@ Parser<ManagedTokenSource>::parse_return_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::BreakExpr>
Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (BREAK);
}
- else
- {
- // minus 6 chars for 5 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 6;
- }
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7688,21 +7666,14 @@ Parser<ManagedTokenSource>::parse_break_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::ContinueExpr>
Parser<ManagedTokenSource>::parse_continue_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (CONTINUE);
}
- else
- {
- // minus 9 chars for 8 in return and a space
- // or just TODO: pass in location data
- locus = lexer.peek_token ()->get_locus () - 9;
- }
// parse label (lifetime) if it exists - create dummy first
AST::Lifetime label = AST::Lifetime::error ();
@@ -7746,11 +7717,11 @@ Parser<ManagedTokenSource>::parse_loop_label ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfExpr>
Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7759,10 +7730,6 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
return nullptr;
}
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// detect accidental if let
if (lexer.peek_token ()->get_id () == LET)
@@ -7908,11 +7875,11 @@ Parser<ManagedTokenSource>::parse_if_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::IfLetExpr>
Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
// TODO: make having outer attributes an error?
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
if (!skip_token (IF))
@@ -7921,10 +7888,6 @@ Parser<ManagedTokenSource>::parse_if_let_expr (AST::AttrVec outer_attrs,
return nullptr;
}
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// detect accidental if expr parsed as if let expr
if (lexer.peek_token ()->get_id () != LET)
@@ -8100,10 +8063,10 @@ template <typename ManagedTokenSource>
std::unique_ptr<AST::LoopExpr>
Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
AST::LoopLabel label,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8118,9 +8081,7 @@ Parser<ManagedTokenSource>::parse_loop_expr (AST::AttrVec outer_attrs,
}
else
{
- if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
- else
+ if (!label.is_error ())
locus = label.get_locus ();
}
@@ -8146,10 +8107,10 @@ template <typename ManagedTokenSource>
std::unique_ptr<AST::WhileLoopExpr>
Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
AST::LoopLabel label,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
if (label.is_error ())
locus = lexer.peek_token ()->get_locus ();
@@ -8164,9 +8125,7 @@ Parser<ManagedTokenSource>::parse_while_loop_expr (AST::AttrVec outer_attrs,
}
else
{
- if (label.is_error ())
- locus = lexer.peek_token ()->get_locus () - 1;
- else
+ if (!label.is_error ())
locus = label.get_locus ();
}
@@ -8423,21 +8382,14 @@ Parser<ManagedTokenSource>::parse_labelled_loop_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::MatchExpr>
Parser<ManagedTokenSource>::parse_match_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (MATCH_TOK);
}
- else
- {
- // TODO: probably just pass in location data as param
- // get current pos then move back 6 - 5 for match, 1 for space
- locus = lexer.peek_token ()->get_locus () - 6;
- }
/* parse scrutinee expression, which is required (and HACK to prevent struct
* expr) */
@@ -8711,16 +8663,14 @@ Parser<ManagedTokenSource>::parse_async_block_expr (AST::AttrVec outer_attrs)
template <typename ManagedTokenSource>
std::unique_ptr<AST::UnsafeBlockExpr>
Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus;
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
skip_token (UNSAFE);
}
- else
- locus = lexer.peek_token ()->get_locus () - 1;
// parse block expression (required)
std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
@@ -8744,19 +8694,14 @@ Parser<ManagedTokenSource>::parse_unsafe_block_expr (AST::AttrVec outer_attrs,
template <typename ManagedTokenSource>
std::unique_ptr<AST::ArrayExpr>
Parser<ManagedTokenSource>::parse_array_expr (AST::AttrVec outer_attrs,
- bool pratt_parse)
+ Location pratt_parsed_loc)
{
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_SQUARE);
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -8933,20 +8878,15 @@ Parser<ManagedTokenSource>::parse_closure_param ()
template <typename ManagedTokenSource>
std::unique_ptr<AST::ExprWithoutBlock>
Parser<ManagedTokenSource>::parse_grouped_or_tuple_expr (
- AST::AttrVec outer_attrs, bool pratt_parse)
+ AST::AttrVec outer_attrs, Location pratt_parsed_loc)
{
// adjustment to allow Pratt parsing to reuse function without copy-paste
- Location locus = Linemap::unknown_location ();
- if (!pratt_parse)
+ Location locus = pratt_parsed_loc;
+ if (locus == Linemap::unknown_location ())
{
locus = lexer.peek_token ()->get_locus ();
-
skip_token (LEFT_PAREN);
}
- else
- {
- locus = lexer.peek_token ()->get_locus () - 1;
- }
// parse optional inner attributes
AST::AttrVec inner_attrs = parse_inner_attributes ();
@@ -12541,7 +12481,7 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
// qualified path
// HACK: add outer attrs to path
AST::QualifiedPathInExpression path
- = parse_qualified_path_in_expression (true);
+ = parse_qualified_path_in_expression (tok->get_locus ());
path.set_outer_attrs (std::move (outer_attrs));
return std::unique_ptr<AST::QualifiedPathInExpression> (
new AST::QualifiedPathInExpression (std::move (path)));
@@ -12584,7 +12524,8 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
new AST::LiteralExpr ("false", AST::Literal::BOOL,
tok->get_type_hint (), {}, tok->get_locus ()));
case LEFT_PAREN:
- return parse_grouped_or_tuple_expr (std::move (outer_attrs), true);
+ return parse_grouped_or_tuple_expr (std::move (outer_attrs),
+ tok->get_locus ());
/*case PLUS: { // unary plus operator
// invoke parse_expr recursively with appropriate priority, etc. for
@@ -12816,41 +12757,43 @@ Parser<ManagedTokenSource>::null_denotation (const_TokenPtr tok,
return parse_range_to_inclusive_expr (tok, std::move (outer_attrs));
case RETURN_TOK:
// FIXME: is this really a null denotation expression?
- return parse_return_expr (std::move (outer_attrs), true);
+ return parse_return_expr (std::move (outer_attrs), tok->get_locus ());
case BREAK:
// FIXME: is this really a null denotation expression?
- return parse_break_expr (std::move (outer_attrs), true);
+ return parse_break_expr (std::move (outer_attrs), tok->get_locus ());
case CONTINUE:
- return parse_continue_expr (std::move (outer_attrs), true);
+ return parse_continue_expr (std::move (outer_attrs), tok->get_locus ());
case LEFT_CURLY:
// ok - this is an expression with block for once.
- return parse_block_expr (std::move (outer_attrs), true);
+ return parse_block_expr (std::move (outer_attrs), tok->get_locus ());
case IF:
// if or if let, so more lookahead to find out
if (lexer.peek_token (1)->get_id () == LET)
{
// if let expr
- return parse_if_let_expr (std::move (outer_attrs), true);
+ return parse_if_let_expr (std::move (outer_attrs), tok->get_locus ());
}
else
{
// if expr
- return parse_if_expr (std::move (outer_attrs), true);
+ return parse_if_expr (std::move (outer_attrs), tok->get_locus ());
}
case LOOP:
return parse_loop_expr (std::move (outer_attrs), AST::LoopLabel::error (),
- true);
+ tok->get_locus ());
case WHILE:
return parse_while_loop_expr (std::move (outer_attrs),
- AST::LoopLabel::error (), true);
+ AST::LoopLabel::error (),
+ tok->get_locus ());
case MATCH_TOK:
// also an expression with block
- return parse_match_expr (std::move (outer_attrs), true);
+ return parse_match_expr (std::move (outer_attrs), tok->get_locus ());
case LEFT_SQUARE:
// array definition expr (not indexing)
- return parse_array_expr (std::move (outer_attrs), true);
+ return parse_array_expr (std::move (outer_attrs), tok->get_locus ());
case UNSAFE:
- return parse_unsafe_block_expr (std::move (outer_attrs), true);
+ return parse_unsafe_block_expr (std::move (outer_attrs),
+ tok->get_locus ());
default:
if (!restrictions.expr_can_be_null)
add_error (Error (tok->get_locus (),
diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
index 3920893faf6..86e0d2a6f83 100644
--- a/gcc/rust/parse/rust-parse.h
+++ b/gcc/rust/parse/rust-parse.h
@@ -121,8 +121,14 @@ private:
AST::PathInExpression parse_path_in_expression ();
AST::PathExprSegment parse_path_expr_segment ();
AST::QualifiedPathInExpression
- parse_qualified_path_in_expression (bool pratt_parse = false);
- AST::QualifiedPathType parse_qualified_path_type (bool pratt_parse = false);
+ // When given a pratt_parsed_loc, use it as the location of the
+ // first token parsed in the expression (the parsing of that first
+ // token should be skipped).
+ parse_qualified_path_in_expression (Location pratt_parsed_loc
+ = Linemap::unknown_location ());
+ AST::QualifiedPathType
+ parse_qualified_path_type (Location pratt_parsed_loc
+ = Linemap::unknown_location ());
AST::QualifiedPathInType parse_qualified_path_in_type ();
// Token tree or macro related
@@ -469,32 +475,36 @@ private:
parse_expr_with_block (AST::AttrVec outer_attrs);
std::unique_ptr<AST::ExprWithoutBlock>
parse_expr_without_block (AST::AttrVec outer_attrs = AST::AttrVec ());
- std::unique_ptr<AST::BlockExpr> parse_block_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfExpr> parse_if_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::IfLetExpr> parse_if_let_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ // When given a pratt_parsed_loc, use it as the location of the
+ // first token parsed in the expression (the parsing of that first
+ // token should be skipped).
+ std::unique_ptr<AST::BlockExpr>
+ parse_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
+ std::unique_ptr<AST::IfExpr>
+ parse_if_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
+ std::unique_ptr<AST::IfLetExpr>
+ parse_if_let_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::LoopExpr>
parse_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::WhileLoopExpr>
parse_while_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
std::unique_ptr<AST::WhileLetLoopExpr>
parse_while_let_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
std::unique_ptr<AST::ForLoopExpr>
parse_for_loop_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
AST::LoopLabel label = AST::LoopLabel::error ());
- std::unique_ptr<AST::MatchExpr> parse_match_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ std::unique_ptr<AST::MatchExpr>
+ parse_match_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
AST::MatchArm parse_match_arm ();
std::vector<std::unique_ptr<AST::Pattern> >
parse_match_arm_patterns (TokenId end_token_id);
@@ -510,24 +520,30 @@ private:
AST::ClosureParam parse_closure_param ();
std::unique_ptr<AST::LiteralExpr> parse_literal_expr (AST::AttrVec outer_attrs
= AST::AttrVec ());
- std::unique_ptr<AST::ReturnExpr> parse_return_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::BreakExpr> parse_break_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ // When given a pratt_parsed_loc, use it as the location of the
+ // first token parsed in the expression (the parsing of that first
+ // token should be skipped).
+ std::unique_ptr<AST::ReturnExpr>
+ parse_return_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
+ std::unique_ptr<AST::BreakExpr>
+ parse_break_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::ContinueExpr>
parse_continue_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
std::unique_ptr<AST::UnsafeBlockExpr>
parse_unsafe_block_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
- std::unique_ptr<AST::ArrayExpr> parse_array_expr (AST::AttrVec outer_attrs
- = AST::AttrVec (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
+ std::unique_ptr<AST::ArrayExpr>
+ parse_array_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
+ Location pratt_parsed_loc = Linemap::unknown_location ());
std::unique_ptr<AST::ExprWithoutBlock>
parse_grouped_or_tuple_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
- bool pratt_parse = false);
+ Location pratt_parsed_loc
+ = Linemap::unknown_location ());
std::unique_ptr<AST::StructExprField> parse_struct_expr_field ();
bool will_be_expr_with_block ();
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-08-09 14:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 22:13 [PATCH] Pass pratt parsed token to expr parser functions to fix expr locus Mark Wielaard
2021-07-29 1:25 ` The Other
2021-07-29 10:55 ` Mark Wielaard
2021-07-29 15:18 ` Thomas Schwinge
2021-07-30 1:03 ` Mark Wielaard
2021-08-05 19:37 ` Mark Wielaard
2021-08-06 15:23 ` Philip Herron
2021-08-07 1:01 ` Mark Wielaard
2021-08-08 11:53 ` Philip Herron
2021-08-09 14:24 ` Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).