Improved Expr parser errors

gcc/rust/ast/rust-ast-collector.cc

@@ -1477,7 +1477,7 @@ TokenCollector::visit (BreakExpr &expr)
     if (expr.has_label ())
       visit (expr.get_label_unchecked ());
     if (expr.has_break_expr ())
-      visit (expr.get_break_expr ());
+      visit (expr.get_break_expr_unchecked ());
   });
 }
 

gcc/rust/ast/rust-ast-pointer-visitor.cc

@@ -490,7 +490,7 @@ PointerVisitor::visit (AST::BreakExpr &expr)
     visit (expr.get_label_unchecked ());
 
   if (expr.has_break_expr ())
-    reseat (expr.get_break_expr_ptr ());
+    reseat (expr.get_break_expr_ptr_unchecked ());
 }
 
 void

gcc/rust/ast/rust-ast-visitor.cc

@@ -510,7 +510,7 @@ DefaultASTVisitor::visit (AST::BreakExpr &expr)
     visit (expr.get_label_unchecked ());
 
   if (expr.has_break_expr ())
-    visit (expr.get_break_expr ());
+    visit (expr.get_break_expr_unchecked ());
 }
 
 void

gcc/rust/ast/rust-ast.cc

@@ -1649,7 +1649,7 @@ ReturnExpr::as_string () const
   std::string str ("return ");
 
   if (has_returned_expr ())
-    str += return_expr->as_string ();
+    str += get_returned_expr ().as_string ();
 
   return str;
 }
@@ -2233,7 +2233,7 @@ BreakExpr::as_string () const
     str += get_label_unchecked ().as_string () + " ";
 
   if (has_break_expr ())
-    str += break_expr->as_string ();
+    str += get_break_expr_unchecked ().as_string ();
 
   return str;
 }
@@ -3726,15 +3726,16 @@ AttributeParser::parse_path_meta_item ()
       {
 	lexer->skip_token ();
 
-	std::unique_ptr<Expr> expr = parser->parse_expr ();
+	auto expr = parser->parse_expr ();
 
 	// handle error
 	// parse_expr should already emit an error and return nullptr
 	if (!expr)
 	  return nullptr;
 
 	return std::unique_ptr<MetaItemPathExpr> (
-	  new MetaItemPathExpr (std::move (path.value ()), std::move (expr)));
+	  new MetaItemPathExpr (std::move (path.value ()),
+				std::move (expr.value ())));
       }
     case COMMA:
       // just simple path
@@ -3813,7 +3814,7 @@ DelimTokenTree::to_token_stream () const
 std::unique_ptr<MetaItemLitExpr>
 AttributeParser::parse_meta_item_lit ()
 {
-  std::unique_ptr<LiteralExpr> lit_expr = parser->parse_literal_expr ({});
+  auto lit_expr = parser->parse_literal_expr ({});
 
   // TODO: return nullptr instead?
   if (!lit_expr)
@@ -3822,7 +3823,7 @@ AttributeParser::parse_meta_item_lit ()
 		       lexer->peek_token ()->get_locus ()));
 
   return std::unique_ptr<MetaItemLitExpr> (
-    new MetaItemLitExpr (std::move (*lit_expr)));
+    new MetaItemLitExpr (std::move (*lit_expr.value ())));
 }
 
 bool

gcc/rust/ast/rust-desugar-for-loops.cc

@@ -37,7 +37,7 @@ DesugarForLoops::DesugarCtx::make_break_arm ()
     builder.path_in_expression (LangItem::Kind::OPTION_NONE))));
 
   auto break_expr
-    = std::unique_ptr<Expr> (new BreakExpr (tl::nullopt, nullptr, {}, loc));
+    = std::unique_ptr<Expr> (new BreakExpr (tl::nullopt, tl::nullopt, {}, loc));
 
   return MatchCase (std::move (arm), std::move (break_expr));
 }

gcc/rust/ast/rust-desugar-while-let.cc

@@ -36,7 +36,7 @@ DesugarWhileLet::DesugarCtx::make_break_arm ()
   auto arm = builder.match_arm (builder.wildcard ());
 
   auto break_expr
-    = std::unique_ptr<Expr> (new BreakExpr (tl::nullopt, nullptr, {}, loc));
+    = std::unique_ptr<Expr> (new BreakExpr (tl::nullopt, tl::nullopt, {}, loc));
 
   return MatchCase (std::move (arm), std::move (break_expr));
 }

gcc/rust/ast/rust-expr.h

@@ -3192,7 +3192,7 @@ class BreakExpr : public ExprWithoutBlock
 {
   std::vector<Attribute> outer_attrs;
   tl::optional<LoopLabel> label;
-  std::unique_ptr<Expr> break_expr;
+  tl::optional<std::unique_ptr<Expr>> break_expr;
   location_t locus;
 
   // TODO: find another way to store this to save memory?
@@ -3206,25 +3206,27 @@ class BreakExpr : public ExprWithoutBlock
 
   /* Returns whether the break expression has an expression used in the break or
    * not. */
-  bool has_break_expr () const { return break_expr != nullptr; }
+  bool has_break_expr () const { return break_expr.has_value (); }
 
   // Constructor for a break expression
   BreakExpr (tl::optional<LoopLabel> break_label,
-	     std::unique_ptr<Expr> expr_in_break,
+	     tl::optional<std::unique_ptr<Expr>> expr_in_break,
 	     std::vector<Attribute> outer_attribs, location_t locus)
     : outer_attrs (std::move (outer_attribs)), label (std::move (break_label)),
       break_expr (std::move (expr_in_break)), locus (locus)
-  {}
+  {
+    if (this->has_break_expr ())
+      rust_assert (this->break_expr != nullptr);
+  }
 
   // Copy constructor defined to use clone for unique pointer
   BreakExpr (BreakExpr const &other)
     : ExprWithoutBlock (other), outer_attrs (other.outer_attrs),
       label (other.label), locus (other.locus),
       marked_for_strip (other.marked_for_strip)
   {
-    // guard to protect from null pointer dereference
-    if (other.break_expr != nullptr)
-      break_expr = other.break_expr->clone_expr ();
+    if (other.has_break_expr ())
+      break_expr = other.get_break_expr_unchecked ().clone_expr ();
   }
 
   // Overload assignment operator to clone unique pointer
@@ -3237,10 +3239,10 @@ class BreakExpr : public ExprWithoutBlock
     outer_attrs = other.outer_attrs;
 
     // guard to protect from null pointer dereference
-    if (other.break_expr != nullptr)
-      break_expr = other.break_expr->clone_expr ();
+    if (other.has_break_expr ())
+      break_expr = other.get_break_expr_unchecked ().clone_expr ();
     else
-      break_expr = nullptr;
+      break_expr = tl::nullopt;
 
     return *this;
   }
@@ -3258,16 +3260,22 @@ class BreakExpr : public ExprWithoutBlock
   bool is_marked_for_strip () const override { return marked_for_strip; }
 
   // TODO: is this better? Or is a "vis_block" better?
-  Expr &get_break_expr ()
+  Expr &get_break_expr_unchecked ()
   {
     rust_assert (has_break_expr ());
-    return *break_expr;
+    return *break_expr.value ();
   }
 
-  std::unique_ptr<Expr> &get_break_expr_ptr ()
+  const Expr &get_break_expr_unchecked () const
   {
     rust_assert (has_break_expr ());
-    return break_expr;
+    return *break_expr.value ();
+  }
+
+  std::unique_ptr<Expr> &get_break_expr_ptr_unchecked ()
+  {
+    rust_assert (has_break_expr ());
+    return break_expr.value ();
   }
 
   const std::vector<Attribute> &get_outer_attrs () const { return outer_attrs; }
@@ -3831,7 +3839,7 @@ class BoxExpr : public ExprWithoutBlock
 class ReturnExpr : public ExprWithoutBlock
 {
   std::vector<Attribute> outer_attrs;
-  std::unique_ptr<Expr> return_expr;
+  tl::optional<std::unique_ptr<Expr>> return_expr;
   location_t locus;
 
   // TODO: find another way to store this to save memory?
@@ -3842,10 +3850,10 @@ class ReturnExpr : public ExprWithoutBlock
 
   /* Returns whether the object has an expression returned (i.e. not void return
    * type). */
-  bool has_returned_expr () const { return return_expr != nullptr; }
+  bool has_returned_expr () const { return return_expr.has_value (); }
 
   // Constructor for ReturnExpr.
-  ReturnExpr (std::unique_ptr<Expr> returned_expr,
+  ReturnExpr (tl::optional<std::unique_ptr<Expr>> returned_expr,
 	      std::vector<Attribute> outer_attribs, location_t locus)
     : outer_attrs (std::move (outer_attribs)),
       return_expr (std::move (returned_expr)), locus (locus)
@@ -3857,8 +3865,8 @@ class ReturnExpr : public ExprWithoutBlock
       locus (other.locus), marked_for_strip (other.marked_for_strip)
   {
     // guard to protect from null pointer dereference
-    if (other.return_expr != nullptr)
-      return_expr = other.return_expr->clone_expr ();
+    if (other.return_expr)
+      return_expr = other.return_expr.value ()->clone_expr ();
   }
 
   // Overloaded assignment operator to clone return_expr pointer
@@ -3870,10 +3878,10 @@ class ReturnExpr : public ExprWithoutBlock
     outer_attrs = other.outer_attrs;
 
     // guard to protect from null pointer dereference
-    if (other.return_expr != nullptr)
-      return_expr = other.return_expr->clone_expr ();
+    if (other.return_expr)
+      return_expr = other.return_expr.value ()->clone_expr ();
     else
-      return_expr = nullptr;
+      return_expr = tl::nullopt;
 
     return *this;
   }
@@ -3893,14 +3901,20 @@ class ReturnExpr : public ExprWithoutBlock
   // TODO: is this better? Or is a "vis_block" better?
   Expr &get_returned_expr ()
   {
-    rust_assert (return_expr != nullptr);
-    return *return_expr;
+    rust_assert (return_expr);
+    return *return_expr.value ();
+  }
+
+  const Expr &get_returned_expr () const
+  {
+    rust_assert (return_expr);
+    return *return_expr.value ();
   }
 
   std::unique_ptr<Expr> &get_returned_expr_ptr ()
   {
-    rust_assert (return_expr != nullptr);
-    return return_expr;
+    rust_assert (return_expr);
+    return return_expr.value ();
   }
 
   const std::vector<Attribute> &get_outer_attrs () const { return outer_attrs; }

gcc/rust/expand/rust-cfg-strip.cc

@@ -1236,7 +1236,7 @@ CfgStrip::visit (AST::BreakExpr &expr)
    * is the only thing that can be done */
   if (expr.has_break_expr ())
     {
-      auto &break_expr = expr.get_break_expr ();
+      auto &break_expr = expr.get_break_expr_unchecked ();
 
       if (break_expr.is_marked_for_strip ())
 	rust_error_at (break_expr.get_locus (),

gcc/rust/expand/rust-macro-builtins-asm.cc

@@ -326,10 +326,11 @@ parse_reg_operand_in (InlineAsmContext inline_asm_ctx)
 	}
 
       auto expr = parser.parse_expr ();
+      rust_assert (expr);
 
       // TODO: When we've succesfully parse an expr, remember to clone_expr()
       // instead of nullptr
-      AST::InlineAsmOperand::In in (reg, std::move (expr));
+      AST::InlineAsmOperand::In in (reg, std::move (expr.value ()));
       inline_asm_ctx.inline_asm.operands.emplace_back (in, locus);
       return inline_asm_ctx;
     }
@@ -344,16 +345,15 @@ parse_reg_operand_out (InlineAsmContext inline_asm_ctx)
   if (!inline_asm_ctx.is_global_asm () && check_identifier (parser, "out"))
     {
       auto reg = parse_reg (inline_asm_ctx);
-      std::unique_ptr<AST::Expr> expr = parser.parse_expr ();
-
-      rust_assert (expr != nullptr);
+      auto expr = parser.parse_expr ();
+      rust_assert (expr);
 
       /*auto expr_ptr =
 	 std::make_unique<AST::Expr>(AST::LiteralExpr(Literal))*/
 
       // TODO: When we've succesfully parse an expr, remember to clone_expr()
       // instead of nullptr
-      AST::InlineAsmOperand::Out out (reg, false, std::move (expr));
+      AST::InlineAsmOperand::Out out (reg, false, std::move (expr.value ()));
 
       inline_asm_ctx.inline_asm.operands.emplace_back (out, locus);
 
@@ -402,21 +402,21 @@ parse_reg_operand_inout (InlineAsmContext inline_asm_ctx)
 
       // TODO: Is error propogation our top priority, the ? in rust's asm.rs is
       // doing a lot of work.
-      std::unique_ptr<AST::Expr> in_expr = parser.parse_expr ();
-      rust_assert (in_expr != nullptr);
-
-      std::unique_ptr<AST::Expr> out_expr;
+      auto in_expr = parser.parse_expr ();
+      rust_assert (in_expr);
 
       if (parser.skip_token (MATCH_ARROW))
 	{
 	  if (!parser.skip_token (UNDERSCORE))
 	    {
 	      // auto result = parse_format_string (inline_asm_ctx);
 
-	      out_expr = parser.parse_expr ();
+	      auto out_expr = parser.parse_expr ();
+	      rust_assert (out_expr);
 
 	      AST::InlineAsmOperand::SplitInOut splitinout (
-		reg, false, std::move (in_expr), std::move (out_expr));
+		reg, false, std::move (in_expr.value ()),
+		std::move (out_expr.value ()));
 
 	      inline_asm_ctx.inline_asm.operands.emplace_back (splitinout,
 							       locus);
@@ -439,7 +439,8 @@ parse_reg_operand_inout (InlineAsmContext inline_asm_ctx)
 	}
       else
 	{
-	  AST::InlineAsmOperand::InOut inout (reg, false, std::move (in_expr));
+	  AST::InlineAsmOperand::InOut inout (reg, false,
+					      std::move (in_expr.value ()));
 	  inline_asm_ctx.inline_asm.operands.emplace_back (inout, locus);
 	  // https://github.com/rust-lang/rust/blob/a3167859f2fd8ff2241295469876a2b687280bdc/compiler/rustc_builtin_macros/src/asm.rs#L137
 	  // RUST VERSION: ast::InlineAsmOperand::InOut { reg, expr, late: false
@@ -1090,10 +1091,11 @@ parse_llvm_operands (LlvmAsmContext &ctx, std::vector<AST::LlvmOperand> &result)
       ParseRestrictions restrictions;
       restrictions.expr_can_be_null = true;
       auto expr = parser.parse_expr ();
+      rust_assert (expr);
 
       parser.skip_token (RIGHT_PAREN);
 
-      result.emplace_back (constraint, std::move (expr));
+      result.emplace_back (constraint, std::move (expr.value ()));
 
       if (parser.peek_current_token ()->get_id () == COMMA)
 	parser.skip_token (COMMA);

gcc/rust/expand/rust-macro-builtins-format-args.cc

@@ -42,8 +42,9 @@ format_args_parse_expr (location_t invoc_locus, AST::MacroInvocData &invoc,
 			Parser<MacroInvocLexer> &parser,
 			BuiltinMacro macro_kind)
 {
-  std::unique_ptr<AST::Expr> format_expr = parser.parse_expr ();
-  rust_assert (format_expr);
+  auto format_expr_res = parser.parse_expr ();
+  rust_assert (format_expr_res);
+  auto format_expr = std::move (format_expr_res.value ());
 
   if (format_expr->get_expr_kind () == AST::Expr::Kind::MacroInvocation)
     {
@@ -114,7 +115,8 @@ format_args_parse_arguments (AST::MacroInvocData &invoc,
 	  if (!expr)
 	    rust_unreachable ();
 
-	  args.push (AST::FormatArgument::named (ident, std::move (expr)));
+	  args.push (
+	    AST::FormatArgument::named (ident, std::move (expr.value ())));
 	}
       else
 	{
@@ -124,7 +126,7 @@ format_args_parse_arguments (AST::MacroInvocData &invoc,
 	  if (!expr)
 	    rust_unreachable ();
 
-	  args.push (AST::FormatArgument::normal (std::move (expr)));
+	  args.push (AST::FormatArgument::normal (std::move (expr.value ())));
 	}
       // we need to skip commas, don't we?
     }

gcc/rust/expand/rust-macro-builtins-helpers.cc

@@ -151,7 +151,7 @@ try_expand_many_expr (Parser<MacroInvocLexer> &parser,
       auto expr = parser.parse_expr (AST::AttrVec (), restrictions);
       // something must be so wrong that the expression could not be parsed
       rust_assert (expr);
-      result.push_back (std::move (expr));
+      result.push_back (std::move (expr.value ()));
 
       auto next_token = parser.peek_current_token ();
       if (!parser.skip_token (COMMA) && next_token->get_id () != last_token_id)
@@ -188,7 +188,7 @@ parse_single_string_literal (BuiltinMacro kind,
 
   if (parser.peek_current_token ()->get_id () == STRING_LITERAL)
     {
-      lit_expr = parser.parse_literal_expr ();
+      lit_expr = parser.parse_literal_expr ().value ();
       parser.maybe_skip_token (COMMA);
       if (parser.peek_current_token ()->get_id () != last_token_id)
 	{