Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ast: builder: Add Return expression builder #3386

Merged
merged 1 commit into from
Feb 2, 2025
+11 −0
Merged

ast: builder: Add Return expression builder #3386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions gcc/rust/ast/rust-ast-builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ Builder::block (std::vector<std::unique_ptr<Stmt>> &&stmts,
LoopLabel::error (), loc, loc));
}

std::unique_ptr<Expr>
Builder::return_expr (std::unique_ptr<Expr> &&to_return)
{
return std::unique_ptr<Expr> (
new ReturnExpr (std::move (to_return), {}, loc));
Comment on lines +220 to +221
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we use make_unique here ? I believe we can even get rid of our implementation since baseline cpp version has been bumped to cpp14.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried using it and it complains about not finding a matching constructor:

../../gcc/rust/ast/rust-ast-builder.cc:220:39: error: no matching function for call to ‘make_unique<Rust::AST::ReturnExpr>(std::remove_reference<std::unique_ptr<Rust::AST::Expr>&>::type, <brace-enclosed initializer list>, location_t&)’
  220 |   return std::make_unique<ReturnExpr> (std::move (to_return), {}, loc);
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

but maybe I'm missing something

}

std::unique_ptr<Stmt>
Builder::let (std::unique_ptr<Pattern> pattern, std::unique_ptr<Type> type,
std::unique_ptr<Expr> init) const
Expand Down
4 changes: 4 additions & 0 deletions gcc/rust/ast/rust-ast-builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class Builder
std::unique_ptr<Expr> &&tail_expr
= nullptr) const;

/* Create an early return expression with an optional expression */
std::unique_ptr<Expr> return_expr (std::unique_ptr<Expr> &&to_return
= nullptr);
Comment on lines +89 to +90
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::unique_ptr<Expr> && I wish it was an optional instead, this would draw a clear line and we would be able to easily catch rogue null pointers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd need to change the AST for this then, because the ReturnExpr class only keeps a unique_ptr. So the builder's code would look like this:

std::unique_ptr<Expr>
Builder::return_expr (tl::optional<std::unique_ptr<Expr>> &&to_return)
{
  if (to_return)
    return std::unique_ptr<Expr> (
      new ReturnExpr (std::move (*to_return), {}, loc));

  return std::unique_ptr<Expr> (new ReturnExpr (nullptr, {}, loc));
}

which is quite heavy


/* Create a let binding with an optional type and initializer (`let <name> :
* <type> = <init>`) */
std::unique_ptr<Stmt> let (std::unique_ptr<Pattern> pattern,
Expand Down
Loading