gccrs: Change lookup_node_to_hir return type to optional

Previous API was using a boolean and a pointer, this was not practical
and could be replaced with an optional.

gcc/rust/ChangeLog:

	* backend/rust-compile-expr.cc (CompileExpr::visit): Change call to use
	the returned optional.
	(CompileExpr::generate_closure_function): Likewise.
	* backend/rust-compile-resolve-path.cc (ResolvePathRef::resolve):
	Likewise.
	* backend/rust-compile-type.cc (TyTyResolveCompile::visit): Likewise.
	* backend/rust-mangle-v0.cc (v0_path): Likewise.
	* checks/errors/privacy/rust-visibility-resolver.cc:
	Likewise.
	* checks/errors/rust-const-checker.cc (ConstChecker::visit): Likewise.
	* checks/errors/rust-unsafe-checker.cc (UnsafeChecker::visit):
	Likewise.
	* checks/lints/rust-lint-marklive.cc (MarkLive::visit): Likewise.
	(MarkLive::visit_path_segment): Likewise.
	* typecheck/rust-hir-trait-resolve.cc (TraitResolver::resolve_path_to_trait):
	Likewise.
	* typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::resolve_root_path):
	Likewise.
	* typecheck/rust-hir-type-check-type.cc (TypeCheckType::resolve_root_path):
	Likewise.
	(ResolveWhereClauseItem::visit): Likewise.
	* util/rust-hir-map.cc (Mappings::lookup_node_to_hir): Return an
	optional instead of a boolean.
	* util/rust-hir-map.h: Change function prototype to match the new
	return type.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>

13 files changed, 142 insertions, 119 deletions

diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index 52318a9..28b901e 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -30,6 +30,7 @@
 #include "realmpfr.h"
 #include "convert.h"
 #include "print-tree.h"
+#include "rust-system.h"
 
 namespace Rust {
 namespace Compile {
@@ -734,12 +735,14 @@ CompileExpr::visit (HIR::BreakExpr &expr)
 	  return;
 	}
 
-      HirId ref = UNKNOWN_HIRID;
-      if (!ctx->get_mappings ().lookup_node_to_hir (resolved_node_id, &ref))
+      tl::optional<HirId> hid
+	= ctx->get_mappings ().lookup_node_to_hir (resolved_node_id);
+      if (!hid.has_value ())
 	{
 	  rust_fatal_error (expr.get_locus (), "reverse lookup label failure");
 	  return;
 	}
+      auto ref = hid.value ();
 
       tree label = NULL_TREE;
       if (!ctx->lookup_label_decl (ref, &label))
@@ -778,12 +781,14 @@ CompileExpr::visit (HIR::ContinueExpr &expr)
 	  return;
 	}
 
-      HirId ref = UNKNOWN_HIRID;
-      if (!ctx->get_mappings ().lookup_node_to_hir (resolved_node_id, &ref))
+      tl::optional<HirId> hid
+	= ctx->get_mappings ().lookup_node_to_hir (resolved_node_id);
+      if (!hid.has_value ())
 	{
 	  rust_fatal_error (expr.get_locus (), "reverse lookup label failure");
 	  return;
 	}
+      auto ref = hid.value ();
 
       if (!ctx->lookup_label_decl (ref, &label))
 	{
@@ -2176,20 +2181,21 @@ CompileExpr::visit (HIR::ClosureExpr &expr)
   for (const auto &capture : closure_tyty->get_captures ())
     {
       // lookup the HirId
-      HirId ref = UNKNOWN_HIRID;
-      bool ok = ctx->get_mappings ().lookup_node_to_hir (capture, &ref);
-      rust_assert (ok);
-
-      // lookup the var decl
-      Bvariable *var = nullptr;
-      bool found = ctx->lookup_var_decl (ref, &var);
-      rust_assert (found);
-
-      // FIXME
-      // this should bes based on the closure move-ability
-      tree var_expr = var->get_tree (expr.get_locus ());
-      tree val = address_expression (var_expr, expr.get_locus ());
-      vals.push_back (val);
+      if (auto hid = ctx->get_mappings ().lookup_node_to_hir (capture))
+	{
+	  // lookup the var decl
+	  Bvariable *var = nullptr;
+	  bool found = ctx->lookup_var_decl (*hid, &var);
+	  rust_assert (found);
+
+	  // FIXME
+	  // this should bes based on the closure move-ability
+	  tree var_expr = var->get_tree (expr.get_locus ());
+	  tree val = address_expression (var_expr, expr.get_locus ());
+	  vals.push_back (val);
+	}
+      else
+	rust_unreachable ();
     }
 
   translated = Backend::constructor_expression (compiled_closure_tyty, false,
@@ -2246,21 +2252,21 @@ CompileExpr::generate_closure_function (HIR::ClosureExpr &expr,
   for (const auto &capture : closure_tyty.get_captures ())
     {
       // lookup the HirId
-      HirId ref = UNKNOWN_HIRID;
-      bool ok = ctx->get_mappings ().lookup_node_to_hir (capture, &ref);
-      rust_assert (ok);
-
-      // get the assessor
-      tree binding = Backend::struct_field_expression (self_param->get_tree (
-							 expr.get_locus ()),
-						       idx, expr.get_locus ());
-      tree indirection = indirect_expression (binding, expr.get_locus ());
+      if (auto hid = ctx->get_mappings ().lookup_node_to_hir (capture))
+	{
+	  // get the assessor
+	  tree binding = Backend::struct_field_expression (
+	    self_param->get_tree (expr.get_locus ()), idx, expr.get_locus ());
+	  tree indirection = indirect_expression (binding, expr.get_locus ());
 
-      // insert bindings
-      ctx->insert_closure_binding (ref, indirection);
+	  // insert bindings
+	  ctx->insert_closure_binding (*hid, indirection);
 
-      // continue
-      idx++;
+	  // continue
+	  idx++;
+	}
+      else
+	rust_unreachable ();
     }
 
   // args tuple
diff --git a/gcc/rust/backend/rust-compile-resolve-path.cc b/gcc/rust/backend/rust-compile-resolve-path.cc
index 9f999f3..48c8d4e 100644
--- a/gcc/rust/backend/rust-compile-resolve-path.cc
+++ b/gcc/rust/backend/rust-compile-resolve-path.cc
@@ -125,12 +125,14 @@ ResolvePathRef::resolve (const HIR::PathIdentSegment &final_segment,
 						      expr_locus);
     }
 
-  HirId ref;
-  if (!ctx->get_mappings ().lookup_node_to_hir (ref_node_id, &ref))
+  tl::optional<HirId> hid
+    = ctx->get_mappings ().lookup_node_to_hir (ref_node_id);
+  if (!hid.has_value ())
     {
       rust_error_at (expr_locus, "reverse call path lookup failure");
       return error_mark_node;
     }
+  auto ref = hid.value ();
 
   // might be a constant
   tree constant_expr;
diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc
index 6840783..b546a05 100644
--- a/gcc/rust/backend/rust-compile-type.cc
+++ b/gcc/rust/backend/rust-compile-type.cc
@@ -155,9 +155,9 @@ TyTyResolveCompile::visit (const TyTy::ClosureType &type)
   for (const auto &capture : type.get_captures ())
     {
       // lookup the HirId
-      HirId ref = UNKNOWN_HIRID;
-      bool ok = mappings.lookup_node_to_hir (capture, &ref);
-      rust_assert (ok);
+      tl::optional<HirId> hid = mappings.lookup_node_to_hir (capture);
+      rust_assert (hid.has_value ());
+      auto ref = hid.value ();
 
       // lookup the var decl type
       TyTy::BaseType *lookup = nullptr;
diff --git a/gcc/rust/backend/rust-mangle-v0.cc b/gcc/rust/backend/rust-mangle-v0.cc
index 0c85d42..b2f11b1 100644
--- a/gcc/rust/backend/rust-mangle-v0.cc
+++ b/gcc/rust/backend/rust-mangle-v0.cc
@@ -374,14 +374,15 @@ v0_path (Rust::Compile::Context *ctx, const TyTy::BaseType *ty,
   V0Path v0path = {};
 
   cpath.iterate_segs ([&] (const Resolver::CanonicalPath &seg) {
-    HirId hir_id;
-    bool ok = mappings.lookup_node_to_hir (seg.get_node_id (), &hir_id);
-    if (!ok)
+    tl::optional<HirId> hid = mappings.lookup_node_to_hir (seg.get_node_id ());
+    if (!hid.has_value ())
       {
 	// FIXME: generic arg in canonical path? (e.g. <i32> in crate::S<i32>)
 	rust_unreachable ();
       }
 
+    auto hir_id = hid.value ();
+
     HirId parent_impl_id = UNKNOWN_HIRID;
     HIR::ImplItem *impl_item
       = mappings.lookup_hir_implitem (hir_id, &parent_impl_id);
diff --git a/gcc/rust/checks/errors/privacy/rust-visibility-resolver.cc b/gcc/rust/checks/errors/privacy/rust-visibility-resolver.cc
index 6b45fd0..3066fee 100644
--- a/gcc/rust/checks/errors/privacy/rust-visibility-resolver.cc
+++ b/gcc/rust/checks/errors/privacy/rust-visibility-resolver.cc
@@ -71,8 +71,9 @@ VisibilityResolver::resolve_module_path (const HIR::SimplePath &restriction,
   // TODO: For the hint, can we point to the original item's definition if
   // present?
 
-  HirId ref;
-  rust_assert (mappings.lookup_node_to_hir (ref_node_id, &ref));
+  tl::optional<HirId> hid = mappings.lookup_node_to_hir (ref_node_id);
+  rust_assert (hid.has_value ());
+  auto ref = hid.value ();
 
   auto crate = mappings.get_ast_crate (mappings.get_current_crate ());
 
diff --git a/gcc/rust/checks/errors/rust-const-checker.cc b/gcc/rust/checks/errors/rust-const-checker.cc
index bdc5c7b..0a99239 100644
--- a/gcc/rust/checks/errors/rust-const-checker.cc
+++ b/gcc/rust/checks/errors/rust-const-checker.cc
@@ -21,6 +21,7 @@
 #include "rust-hir-expr.h"
 #include "rust-hir-stmt.h"
 #include "rust-hir-item.h"
+#include "rust-system.h"
 
 namespace Rust {
 namespace HIR {
@@ -352,18 +353,22 @@ ConstChecker::visit (CallExpr &expr)
 
   NodeId ast_node_id = expr.get_fnexpr ()->get_mappings ().get_nodeid ();
   NodeId ref_node_id;
-  HirId definition_id;
 
   // We don't care about types here
   if (!resolver.lookup_resolved_name (ast_node_id, &ref_node_id))
     return;
 
-  rust_assert (mappings.lookup_node_to_hir (ref_node_id, &definition_id));
-
-  check_function_call (definition_id, expr.get_locus ());
+  if (auto definition_id = mappings.lookup_node_to_hir (ref_node_id))
+    {
+      check_function_call (*definition_id, expr.get_locus ());
 
-  for (auto &arg : expr.get_arguments ())
-    arg->accept_vis (*this);
+      for (auto &arg : expr.get_arguments ())
+	arg->accept_vis (*this);
+    }
+  else
+    {
+      rust_unreachable ();
+    }
 }
 
 void
diff --git a/gcc/rust/checks/errors/rust-unsafe-checker.cc b/gcc/rust/checks/errors/rust-unsafe-checker.cc
index beea96f..92cf476 100644
--- a/gcc/rust/checks/errors/rust-unsafe-checker.cc
+++ b/gcc/rust/checks/errors/rust-unsafe-checker.cc
@@ -22,6 +22,7 @@
 #include "rust-hir-stmt.h"
 #include "rust-hir-item.h"
 #include "rust-attribute-values.h"
+#include "rust-system.h"
 
 namespace Rust {
 namespace HIR {
@@ -220,14 +221,18 @@ UnsafeChecker::visit (PathInExpression &path)
 {
   NodeId ast_node_id = path.get_mappings ().get_nodeid ();
   NodeId ref_node_id;
-  HirId definition_id;
 
   if (!resolver.lookup_resolved_name (ast_node_id, &ref_node_id))
     return;
 
-  rust_assert (mappings.lookup_node_to_hir (ref_node_id, &definition_id));
-
-  check_use_of_static (definition_id, path.get_locus ());
+  if (auto definition_id = mappings.lookup_node_to_hir (ref_node_id))
+    {
+      check_use_of_static (*definition_id, path.get_locus ());
+    }
+  else
+    {
+      rust_unreachable ();
+    }
 }
 
 void
@@ -415,7 +420,6 @@ UnsafeChecker::visit (CallExpr &expr)
 
   NodeId ast_node_id = expr.get_fnexpr ()->get_mappings ().get_nodeid ();
   NodeId ref_node_id;
-  HirId definition_id;
 
   // There are no unsafe types, and functions are defined in the name resolver.
   // If we can't find the name, then we're dealing with a type and should return
@@ -423,19 +427,24 @@ UnsafeChecker::visit (CallExpr &expr)
   if (!resolver.lookup_resolved_name (ast_node_id, &ref_node_id))
     return;
 
-  rust_assert (mappings.lookup_node_to_hir (ref_node_id, &definition_id));
-
-  // At this point we have the function's HIR Id. There are three checks we
-  // must perform:
-  //     1. The function is an unsafe one
-  //     2. The function is an extern one
-  //     3. The function is marked with a target_feature attribute
-  check_function_call (definition_id, expr.get_locus ());
-  check_function_attr (definition_id, expr.get_locus ());
-
-  if (expr.has_params ())
-    for (auto &arg : expr.get_arguments ())
-      arg->accept_vis (*this);
+  if (auto definition_id = mappings.lookup_node_to_hir (ref_node_id))
+    {
+      // At this point we have the function's HIR Id. There are three checks we
+      // must perform:
+      //     1. The function is an unsafe one
+      //     2. The function is an extern one
+      //     3. The function is marked with a target_feature attribute
+      check_function_call (*definition_id, expr.get_locus ());
+      check_function_attr (*definition_id, expr.get_locus ());
+
+      if (expr.has_params ())
+	for (auto &arg : expr.get_arguments ())
+	  arg->accept_vis (*this);
+    }
+  else
+    {
+      rust_unreachable ();
+    }
 }
 
 void
diff --git a/gcc/rust/checks/lints/rust-lint-marklive.cc b/gcc/rust/checks/lints/rust-lint-marklive.cc
index 60a0800..815fcf5 100644
--- a/gcc/rust/checks/lints/rust-lint-marklive.cc
+++ b/gcc/rust/checks/lints/rust-lint-marklive.cc
@@ -24,6 +24,7 @@
 #include "rust-hir-full.h"
 #include "rust-name-resolver.h"
 #include "rust-immutable-name-resolution-context.h"
+#include "rust-system.h"
 
 namespace Rust {
 namespace Analysis {
@@ -118,9 +119,9 @@ MarkLive::visit (HIR::PathInExpression &expr)
   find_ref_node_id (ast_node_id, ref_node_id);
 
   // node back to HIR
-  HirId ref;
-  bool ok = mappings.lookup_node_to_hir (ref_node_id, &ref);
-  rust_assert (ok);
+  tl::optional<HirId> hid = mappings.lookup_node_to_hir (ref_node_id);
+  rust_assert (hid.has_value ());
+  auto ref = hid.value ();
 
   // it must resolve to some kind of HIR::Item or HIR::InheritImplItem
   HIR::Item *resolved_item = mappings.lookup_hir_item (ref);
@@ -154,10 +155,10 @@ MarkLive::visit (HIR::MethodCallExpr &expr)
   find_ref_node_id (ast_node_id, ref_node_id);
 
   // node back to HIR
-  HirId ref;
-  bool ok = mappings.lookup_node_to_hir (ref_node_id, &ref);
-  rust_assert (ok);
-  mark_hir_id (ref);
+  if (auto hid = mappings.lookup_node_to_hir (ref_node_id))
+    mark_hir_id (*hid);
+  else
+    rust_unreachable ();
 }
 
 bool
@@ -179,11 +180,12 @@ MarkLive::visit_path_segment (HIR::PathExprSegment seg)
       if (!resolver->lookup_resolved_type (ast_node_id, &ref_node_id))
 	return false;
     }
-  HirId ref;
-  bool ok = mappings.lookup_node_to_hir (ref_node_id, &ref);
-  rust_assert (ok);
-  mark_hir_id (ref);
-  return true;
+  if (auto hid = mappings.lookup_node_to_hir (ref_node_id))
+    {
+      mark_hir_id (*hid);
+      return true;
+    }
+  rust_unreachable ();
 }
 
 void
@@ -253,10 +255,10 @@ MarkLive::visit (HIR::TypeAlias &alias)
   NodeId ast_node_id;
   resolver->lookup_resolved_type (
     alias.get_type_aliased ()->get_mappings ().get_nodeid (), &ast_node_id);
-  HirId hir_id;
-  bool ok = mappings.lookup_node_to_hir (ast_node_id, &hir_id);
-  rust_assert (ok);
-  mark_hir_id (hir_id);
+  if (auto hid = mappings.lookup_node_to_hir (ast_node_id))
+    mark_hir_id (*hid);
+  else
+    rust_unreachable ();
 }
 
 void
diff --git a/gcc/rust/typecheck/rust-hir-trait-resolve.cc b/gcc/rust/typecheck/rust-hir-trait-resolve.cc
index d3e3167..b8075ee 100644
--- a/gcc/rust/typecheck/rust-hir-trait-resolve.cc
+++ b/gcc/rust/typecheck/rust-hir-trait-resolve.cc
@@ -117,19 +117,18 @@ TraitResolver::resolve_path_to_trait (const HIR::TypePath &path,
       return false;
     }
 
-  HirId hir_node = UNKNOWN_HIRID;
-  if (!mappings.lookup_node_to_hir (ref, &hir_node))
+  if (auto hid = mappings.lookup_node_to_hir (ref))
     {
-      rust_error_at (path.get_locus (), "Failed to resolve path to hir-id");
-      return false;
-    }
-
-  HIR::Item *resolved_item = mappings.lookup_hir_item (hir_node);
-  rust_assert (resolved_item != nullptr);
-  rust_assert (resolved_item->get_item_kind () == HIR::Item::ItemKind::Trait);
-  *resolved = static_cast<HIR::Trait *> (resolved_item);
+      HIR::Item *resolved_item = mappings.lookup_hir_item (hid.value ());
+      rust_assert (resolved_item != nullptr);
+      rust_assert (resolved_item->get_item_kind ()
+		   == HIR::Item::ItemKind::Trait);
+      *resolved = static_cast<HIR::Trait *> (resolved_item);
 
-  return true;
+      return true;
+    }
+  rust_error_at (path.get_locus (), "Failed to resolve path to hir-id");
+  return false;
 }
 
 TraitReference *
diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc
index b4aedda..0cc9f0e 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-path.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc
@@ -229,8 +229,8 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
 	}
 
       // node back to HIR
-      HirId ref;
-      if (!mappings.lookup_node_to_hir (ref_node_id, &ref))
+      tl::optional<HirId> hid = mappings.lookup_node_to_hir (ref_node_id);
+      if (!hid.has_value ())
 	{
 	  rust_error_at (seg.get_locus (), "456 reverse lookup failure");
 	  rust_debug_loc (seg.get_locus (),
@@ -241,6 +241,7 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
 
 	  return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
 	}
+      auto ref = hid.value ();
 
       auto seg_is_module = (nullptr != mappings.lookup_module (ref));
       auto seg_is_crate = mappings.is_local_hirid_crate (ref);
diff --git a/gcc/rust/typecheck/rust-hir-type-check-type.cc b/gcc/rust/typecheck/rust-hir-type-check-type.cc
index cfcea31..eb4abbb 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-type.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-type.cc
@@ -372,8 +372,8 @@ TypeCheckType::resolve_root_path (HIR::TypePath &path, size_t *offset,
 	}
 
       // node back to HIR
-      HirId ref = UNKNOWN_HIRID;
-      if (!mappings.lookup_node_to_hir (ref_node_id, &ref))
+      tl::optional<HirId> hid = mappings.lookup_node_to_hir (ref_node_id);
+      if (!hid.has_value ())
 	{
 	  if (is_root)
 	    {
@@ -389,6 +389,7 @@ TypeCheckType::resolve_root_path (HIR::TypePath &path, size_t *offset,
 
 	  return root_tyty;
 	}
+      auto ref = hid.value ();
 
       auto seg_is_module = (nullptr != mappings.lookup_module (ref));
       auto seg_is_crate = mappings.is_local_hirid_crate (ref);
@@ -997,27 +998,24 @@ ResolveWhereClauseItem::visit (HIR::TypeBoundWhereClauseItem &item)
     }
 
   // node back to HIR
-  HirId ref;
-  if (!mappings.lookup_node_to_hir (ref_node_id, &ref))
+  if (auto hid = mappings.lookup_node_to_hir (ref_node_id))
     {
-      // FIXME
-      rust_error_at (UNDEF_LOCATION, "where-clause reverse lookup failure");
-      return;
-    }
+      // the base reference for this name _must_ have a type set
+      TyTy::BaseType *lookup;
+      if (!context->lookup_type (*hid, &lookup))
+	{
+	  rust_error_at (mappings.lookup_location (*hid),
+			 "Failed to resolve where-clause binding type: %s",
+			 binding_type_path->as_string ().c_str ());
+	  return;
+	}
 
-  // the base reference for this name _must_ have a type set
-  TyTy::BaseType *lookup;
-  if (!context->lookup_type (ref, &lookup))
-    {
-      rust_error_at (mappings.lookup_location (ref),
-		     "Failed to resolve where-clause binding type: %s",
-		     binding_type_path->as_string ().c_str ());
+      // FIXME
+      // rust_assert (binding->is_equal (*lookup));
+      lookup->inherit_bounds (specified_bounds);
       return;
     }
-
-  // FIXME
-  // rust_assert (binding->is_equal (*lookup));
-  lookup->inherit_bounds (specified_bounds);
+  rust_error_at (UNDEF_LOCATION, "where-clause reverse lookup failure");
 }
 
 } // namespace Resolver
diff --git a/gcc/rust/util/rust-hir-map.cc b/gcc/rust/util/rust-hir-map.cc
index 113e316..78ce17d 100644
--- a/gcc/rust/util/rust-hir-map.cc
+++ b/gcc/rust/util/rust-hir-map.cc
@@ -769,15 +769,14 @@ Mappings::insert_node_to_hir (NodeId id, HirId ref)
   hirIdToNodeMappings[ref] = id;
 }
 
-bool
-Mappings::lookup_node_to_hir (NodeId id, HirId *ref)
+tl::optional<HirId>
+Mappings::lookup_node_to_hir (NodeId id)
 {
   auto it = nodeIdToHirMappings.find (id);
   if (it == nodeIdToHirMappings.end ())
-    return false;
+    return tl::nullopt;
 
-  *ref = it->second;
-  return true;
+  return {it->second};
 }
 
 bool
diff --git a/gcc/rust/util/rust-hir-map.h b/gcc/rust/util/rust-hir-map.h
index b238f14..c2b3e27 100644
--- a/gcc/rust/util/rust-hir-map.h
+++ b/gcc/rust/util/rust-hir-map.h
@@ -170,7 +170,7 @@ public:
 				    std::function<bool (HIR::Item *)> cb);
 
   void insert_node_to_hir (NodeId id, HirId ref);
-  bool lookup_node_to_hir (NodeId id, HirId *ref);
+  tl::optional<HirId> lookup_node_to_hir (NodeId id);
   bool lookup_hir_to_node (HirId id, NodeId *ref);
 
   void insert_location (HirId id, location_t locus);