Use tl::expected in the parser to avoid error state

We made heavy use of error state within some AST node and it was the
source of multiple errors, and caused confusion with (null) pointers.
This commit removes some error state use within our parser in an attempt
to remove those error states later.

gcc/rust/ChangeLog:

	* parse/rust-parse-impl.h (Parser::parse_inner_attributes): Change
	return type to avoid empty/error values that may break invariants in
	the AST.
	(Parser::parse_inner_attribute): Likewise.
	(Parser::parse_outer_attribute): Likewise.
	(Parser::parse_outer_attributes): Likewise.
	(Parser::parse_attribute_body): Likewise.
	(Parser::parse_simple_path): Likewise.
	(Parser::parse_macro_invocation): Likewise.
	(Parser::parse_visibility): Likewise.
	(Parser::parse_use_tree): Likewise.
	(Parser::parse_delim_token_tree): Likewise.
	(Parser::parse_identifier_or_keyword_token): Likewise.
	(Parser::parse_token_tree): Likewise.
	(Parser::parse_macro_rules_def): Likewise.
	(Parser::parse_decl_macro_def): Likewise.
	(Parser::parse_macro_invocation): Likewise.
	(Parser::parse_macro_rule): Likewise.
	(Parser::parse_macro_matcher): Likewise.
	(Parser::parse_type_path_segment): Likewise.
	(Parser::parse_path_expr_segment): Likewise.
	(Parser::parse_type): Likewise.
	(Parser::parse_type_no_bounds): Likewise.
	(is_simple_path_segment): Move to utility file.
	(token_id_matches_delims): Likewise.
	(is_likely_path_next): Remove unused function.
	(Parser::parse_attr_input): Return a structure instead of a tuple.
	* expand/rust-macro-builtins-offset-of.cc: Adapt call to expected.
	* ast/rust-ast.cc (AttributeParser::parse_path_meta_item): Use empty
	vector when an error is encountered.
	* parse/rust-parse.h: Update prototypes.
	* parse/rust-parse-utils.h: New file.
	* parse/rust-parse-error.h: New file.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>

Author: P-E-P

gcc/rust/ast/rust-ast.cc

@@ -3478,7 +3478,9 @@ Module::load_items ()
 
   // we need to parse any possible inner attributes for this module
   inner_attrs = parser.parse_inner_attributes ();
-  auto parsed_items = parser.parse_items ();
+  auto parsed_items = parser.parse_items ().value_or (
+    std::vector<std::unique_ptr<AST::Item>>{});
+
   for (const auto &error : parser.get_errors ())
     error.emit ();
 
@@ -3693,8 +3695,8 @@ AttributeParser::is_end_meta_item_tok (TokenId id) const
 std::unique_ptr<MetaItem>
 AttributeParser::parse_path_meta_item ()
 {
-  SimplePath path = parser->parse_simple_path ();
-  if (path.is_empty ())
+  auto path = parser->parse_simple_path ();
+  if (!path)
     {
       rust_error_at (lexer->peek_token ()->get_locus (),
 		     "failed to parse simple path in attribute");
@@ -3709,7 +3711,7 @@ AttributeParser::parse_path_meta_item ()
 	  = parse_meta_item_seq ();
 
 	return std::unique_ptr<MetaItemSeq> (
-	  new MetaItemSeq (std::move (path), std::move (meta_items)));
+	  new MetaItemSeq (std::move (path.value ()), std::move (meta_items)));
       }
     case EQUAL:
       {
@@ -3723,12 +3725,12 @@ AttributeParser::parse_path_meta_item ()
 	  return nullptr;
 
 	return std::unique_ptr<MetaItemPathExpr> (
-	  new MetaItemPathExpr (std::move (path), std::move (expr)));
+	  new MetaItemPathExpr (std::move (path.value ()), std::move (expr)));
       }
     case COMMA:
       // just simple path
       return std::unique_ptr<MetaItemPath> (
-	new MetaItemPath (std::move (path)));
+	new MetaItemPath (std::move (path.value ())));
     default:
       rust_error_at (lexer->peek_token ()->get_locus (),
 		     "unrecognised token '%s' in meta item",

gcc/rust/ast/rust-ast.h

@@ -477,38 +477,21 @@ struct Visibility
   SimplePath in_path;
   location_t locus;
 
-  // should this store location info?
-
-public:
-  // Creates a Visibility - TODO make constructor protected or private?
   Visibility (VisType vis_type, SimplePath in_path, location_t locus)
     : vis_type (vis_type), in_path (std::move (in_path)), locus (locus)
   {}
 
+public:
   VisType get_vis_type () const { return vis_type; }
 
-  // Returns whether visibility is in an error state.
-  bool is_error () const
-  {
-    return vis_type == PUB_IN_PATH && in_path.is_empty ();
-  }
-
   // Returns whether a visibility has a path
-  bool has_path () const { return !is_error () && vis_type >= PUB_CRATE; }
+  bool has_path () const { return vis_type >= PUB_CRATE; }
 
   // Returns whether visibility is public or not.
-  bool is_public () const { return vis_type != PRIV && !is_error (); }
+  bool is_public () const { return vis_type != PRIV; }
 
   location_t get_locus () const { return locus; }
 
-  // empty?
-  // Creates an error visibility.
-  static Visibility create_error ()
-  {
-    return Visibility (PUB_IN_PATH, SimplePath::create_empty (),
-		       UNDEF_LOCATION);
-  }
-
   // Unique pointer custom clone function
   /*std::unique_ptr<Visibility> clone_visibility() const {
       return std::unique_ptr<Visibility>(clone_visibility_impl());

gcc/rust/ast/rust-item.h

@@ -814,7 +814,7 @@ class Module : public VisItem
   // Loaded module constructor, with items
   Module (Identifier name, location_t locus,
 	  std::vector<std::unique_ptr<Item>> items,
-	  Visibility visibility = Visibility::create_error (),
+	  Visibility visibility = Visibility::create_private (),
 	  Unsafety safety = Unsafety::Normal,
 	  std::vector<Attribute> inner_attrs = std::vector<Attribute> (),
 	  std::vector<Attribute> outer_attrs = std::vector<Attribute> ())
@@ -1743,7 +1743,7 @@ class StructField
   bool has_outer_attributes () const { return !outer_attrs.empty (); }
 
   // Returns whether struct field has a non-private (non-default) visibility.
-  bool has_visibility () const { return !visibility.is_error (); }
+  bool has_visibility () const { return true; }
 
   StructField (Identifier field_name, std::unique_ptr<Type> field_type,
 	       Visibility vis, location_t locus,
@@ -1797,8 +1797,8 @@ class StructField
   // Creates an error state struct field.
   static StructField create_error ()
   {
-    return StructField (std::string (""), nullptr, Visibility::create_error (),
-			UNDEF_LOCATION);
+    return StructField (std::string (""), nullptr,
+			Visibility::create_private (), UNDEF_LOCATION);
   }
 
   std::string as_string () const;
@@ -1902,7 +1902,7 @@ class TupleField
 
   /* Returns whether tuple field has a non-default visibility (i.e. a public
    * one) */
-  bool has_visibility () const { return !visibility.is_error (); }
+  bool has_visibility () const { return true; }
 
   // Complete constructor
   TupleField (std::unique_ptr<Type> field_type, Visibility vis,
@@ -1952,7 +1952,7 @@ class TupleField
   // Creates an error state tuple field.
   static TupleField create_error ()
   {
-    return TupleField (nullptr, Visibility::create_error (), UNDEF_LOCATION);
+    return TupleField (nullptr, Visibility::create_private (), UNDEF_LOCATION);
   }
 
   std::string as_string () const;
@@ -3368,7 +3368,7 @@ class ExternalTypeItem : public ExternalItem
   bool has_outer_attrs () const { return !outer_attrs.empty (); }
 
   // Returns whether item has non-default visibility.
-  bool has_visibility () const { return !visibility.is_error (); }
+  bool has_visibility () const { return true; }
 
   location_t get_locus () const { return locus; }
 
@@ -3460,7 +3460,7 @@ class ExternalStaticItem : public ExternalItem
   bool has_outer_attrs () const { return !outer_attrs.empty (); }
 
   // Returns whether item has non-default visibility.
-  bool has_visibility () const { return !visibility.is_error (); }
+  bool has_visibility () const { return true; }
 
   location_t get_locus () const { return locus; }
 

gcc/rust/ast/rust-macro.h

@@ -529,7 +529,7 @@ class MacroRulesDefinition : public VisItem
     return std::make_unique<MacroRulesDefinition> (
       MacroRulesDefinition (rule_name, delim_type, rules, outer_attrs, locus,
 			    AST::MacroRulesDefinition::MacroKind::MBE,
-			    AST::Visibility::create_error ()));
+			    AST::Visibility::create_private ()));
   }
 
   static std::unique_ptr<MacroRulesDefinition>

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

@@ -248,7 +248,8 @@ MacroBuiltin::include_handler (location_t invoc_locus,
   std::vector<std::unique_ptr<AST::Item>> parsed_items{};
 
   if (is_semicoloned)
-    parsed_items = parser.parse_items ();
+    parsed_items = parser.parse_items ().value_or (
+      std::vector<std::unique_ptr<AST::Item>>{});
   else
     parsed_expr = parser.parse_expr ();
 

gcc/rust/expand/rust-macro-builtins-offset-of.cc

@@ -56,7 +56,7 @@ MacroBuiltin::offset_of_handler (location_t invoc_locus,
   parser.skip_token (COMMA);
 
   auto field_tok = parser.parse_identifier_or_keyword_token ();
-  auto invalid_field = !field_tok || !field_tok->should_have_str ();
+  auto invalid_field = !field_tok || !field_tok.value ()->should_have_str ();
 
   if (invalid_field)
     rust_error_at (invoc_locus, "could not parse field argument for %qs",
@@ -65,7 +65,7 @@ MacroBuiltin::offset_of_handler (location_t invoc_locus,
   if (!type || invalid_field)
     return tl::nullopt;
 
-  auto field = Identifier (field_tok->get_str ());
+  auto field = Identifier (field_tok.value ()->get_str ());
 
   // FIXME: Do we need to do anything to handle the optional comma at the end?
   parser.maybe_skip_token (COMMA);

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

@@ -858,7 +858,9 @@ transcribe_many_items (Parser<MacroInvocLexer> &parser, TokenId &delimiter)
 {
   return parse_many (parser, delimiter, [&parser] () {
     auto item = parser.parse_item (true);
-    return AST::SingleASTNode (std::move (item));
+    if (!item)
+      return AST::SingleASTNode (std::unique_ptr<AST::Item> (nullptr));
+    return AST::SingleASTNode (std::move (item.value ()));
   });
 }
 
@@ -1188,9 +1190,9 @@ MacroExpander::parse_proc_macro_output (ProcMacro::TokenStream ts)
       while (lex.peek_token ()->get_id () != END_OF_FILE)
 	{
 	  auto result = parser.parse_item (false);
-	  if (result == nullptr)
+	  if (!result)
 	    break;
-	  nodes.emplace_back (std::move (result));
+	  nodes.emplace_back (std::move (result.value ()));
 	}
       break;
     case ContextType::STMT:

gcc/rust/hir/rust-ast-lower.cc

@@ -38,8 +38,6 @@ translate_visibility (const AST::Visibility &vis)
   // the AST vis is an error?
   // FIXME: We need to add a `create_private()` static function to the
   // AST::Visibility class and use it when the vis is empty in the parser...
-  if (vis.is_error ())
-    return Visibility::create_error ();
 
   switch (vis.get_vis_type ())
     {
@@ -57,7 +55,7 @@ translate_visibility (const AST::Visibility &vis)
       break;
     }
 
-  return Visibility::create_error ();
+  rust_unreachable ();
 }
 
 ASTLowering::ASTLowering (AST::Crate &astCrate) : astCrate (astCrate) {}