Merge #1358

1358: Disambiguate generic args r=CohenArthur a=CohenArthur

Needs #1355. Only review the last commit.  The next steps are to typecheck const generics and then lower them to TREE

This removes all the hacks previously introduced to resolve ambiguous
generic args as types, and adds proper disambiguation.

The algorithm is as follows:

is that name referring to a type?
-> disambiguate to a type
is that name referring to a value?
-> disambiguate to a const value
else
-> disambiguate to type

Since types are the default expected behavior, this allows us to report
type errors properly during typechecking.




Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

7 files changed, 140 insertions, 51 deletions

diff --git a/gcc/rust/expand/rust-attribute-visitor.cc b/gcc/rust/expand/rust-attribute-visitor.cc
index 749f7eb..9332bb6 100644
--- a/gcc/rust/expand/rust-attribute-visitor.cc
+++ b/gcc/rust/expand/rust-attribute-visitor.cc
@@ -134,20 +134,35 @@ AttrVisitor::expand_generic_args (AST::GenericArgs &args)
   // expand type args - strip sub-types only
   for (auto &arg : args.get_generic_args ())
     {
-      // FIXME: Arthur: Another ugly hack while waiting for disambiguation
-      if (arg.get_kind () == AST::GenericArg::Kind::Either)
-	arg = arg.disambiguate_to_type ();
-
-      if (arg.get_kind () == AST::GenericArg::Kind::Type)
+      switch (arg.get_kind ())
 	{
-	  auto &type = arg.get_type ();
-
-	  type->accept_vis (*this);
-	  maybe_expand_type (type);
-
-	  if (type->is_marked_for_strip ())
-	    rust_error_at (type->get_locus (),
-			   "cannot strip type in this position");
+	  case AST::GenericArg::Kind::Type: {
+	    auto &type = arg.get_type ();
+	    type->accept_vis (*this);
+	    maybe_expand_type (type);
+
+	    if (type->is_marked_for_strip ())
+	      rust_error_at (type->get_locus (),
+			     "cannot strip type in this position");
+	    break;
+	  }
+	  case AST::GenericArg::Kind::Const: {
+	    auto &expr = arg.get_expression ();
+	    expr->accept_vis (*this);
+	    maybe_expand_expr (expr);
+
+	    if (expr->is_marked_for_strip ())
+	      rust_error_at (expr->get_locus (),
+			     "cannot strip expression in this position");
+	    break;
+	  }
+	default:
+	  break;
+	  // FIXME: Figure out what to do here if there is ambiguity. Since the
+	  // resolver comes after the expansion, we need to figure out a way to
+	  // strip ambiguous values here
+	  // TODO: Arthur: Probably add a `mark_as_strip` method to `GenericArg`
+	  // or something. This would clean up this whole thing
 	}
     }
 
diff --git a/gcc/rust/hir/rust-ast-lower-base.cc b/gcc/rust/hir/rust-ast-lower-base.cc
index 3db7dc3..e31b95b 100644
--- a/gcc/rust/hir/rust-ast-lower-base.cc
+++ b/gcc/rust/hir/rust-ast-lower-base.cc
@@ -623,13 +623,6 @@ ASTLoweringBase::lower_generic_args (AST::GenericArgs &args)
 				    expr->get_locus ()));
 	    break;
 	  }
-	  // FIXME: Arthur: Other horrible hack waiting for disambiguation
-	  case AST::GenericArg::Kind::Either: {
-	    arg = arg.disambiguate_to_type ();
-	    auto type = ASTLoweringType::translate (arg.get_type ().get ());
-	    type_args.emplace_back (std::unique_ptr<HIR::Type> (type));
-	    break;
-	  }
 	default:
 	  gcc_unreachable ();
 	}
diff --git a/gcc/rust/hir/rust-ast-lower-type.h b/gcc/rust/hir/rust-ast-lower-type.h
index 317fe85..46f765a 100644
--- a/gcc/rust/hir/rust-ast-lower-type.h
+++ b/gcc/rust/hir/rust-ast-lower-type.h
@@ -356,12 +356,9 @@ public:
 				   mappings->get_next_localdef_id (crate_num));
 
     auto type = ASTLoweringType::translate (param.get_type ().get ());
-    // FIXME: Arthur: Remove the second guard once we disambiguate in the
-    // resolveer
+
     HIR::Expr *default_expr = nullptr;
-    if (param.has_default_value ()
-	&& param.get_default_value ().get_kind ()
-	     == AST::GenericArg::Kind::Const)
+    if (param.has_default_value ())
       default_expr = ASTLoweringExpr::translate (
 	param.get_default_value ().get_expression ().get ());
 
diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.cc b/gcc/rust/resolve/rust-ast-resolve-expr.cc
index 2553c85..4cc4e26 100644
--- a/gcc/rust/resolve/rust-ast-resolve-expr.cc
+++ b/gcc/rust/resolve/rust-ast-resolve-expr.cc
@@ -87,7 +87,7 @@ ResolveExpr::visit (AST::MethodCallExpr &expr)
   if (expr.get_method_name ().has_generic_args ())
     {
       AST::GenericArgs &args = expr.get_method_name ().get_generic_args ();
-      ResolveGenericArgs::go (args);
+      ResolveGenericArgs::go (args, prefix, canonical_prefix);
     }
 
   auto const &in_params = expr.get_params ();
diff --git a/gcc/rust/resolve/rust-ast-resolve-type.cc b/gcc/rust/resolve/rust-ast-resolve-type.cc
index 04183a4..a8931ce 100644
--- a/gcc/rust/resolve/rust-ast-resolve-type.cc
+++ b/gcc/rust/resolve/rust-ast-resolve-type.cc
@@ -384,6 +384,7 @@ ResolveTypeToCanonicalPath::visit (AST::TypePath &path)
 	    std::vector<CanonicalPath> args;
 	    if (s->has_generic_args ())
 	      {
+		ResolveGenericArgs::go (s->get_generic_args ());
 		for (auto &generic : s->get_generic_args ().get_generic_args ())
 		  {
 		    // FIXME: What do we want to do here in case there is a
@@ -391,25 +392,13 @@ ResolveTypeToCanonicalPath::visit (AST::TypePath &path)
 		    // TODO: At that point, will all generics have been
 		    // disambiguated? Can we thus canonical resolve types and
 		    // const and `gcc_unreachable` on ambiguous types?
-		    //
-		    // FIXME: Arthur: This is an ugly hack to resolve just as
-		    // much as before despite not handling ambiguity yet. The
-		    // calls to `clone_type` will be removed.
-		    std::unique_ptr<AST::Type> gt = nullptr;
-
+		    // This is probably fine as we just want to canonicalize
+		    // types, right?
 		    if (generic.get_kind () == AST::GenericArg::Kind::Type)
-		      gt = generic.get_type ()->clone_type ();
-		    else if (generic.get_kind ()
-			     == AST::GenericArg::Kind::Either)
-		      gt = generic.disambiguate_to_type ()
-			     .get_type ()
-			     ->clone_type ();
-
-		    if (gt)
 		      {
 			CanonicalPath arg = CanonicalPath::create_empty ();
-			bool ok
-			  = ResolveTypeToCanonicalPath::go (gt.get (), arg);
+			bool ok = ResolveTypeToCanonicalPath::go (
+			  generic.get_type ().get (), arg);
 			if (ok)
 			  args.push_back (std::move (arg));
 		      }
@@ -492,20 +481,79 @@ ResolveTypeToCanonicalPath::ResolveTypeToCanonicalPath ()
   : ResolverBase (), result (CanonicalPath::create_empty ())
 {}
 
+bool
+ResolveGenericArgs::is_const_value_name (const CanonicalPath &path)
+{
+  NodeId resolved;
+  auto found = resolver->get_name_scope ().lookup (path, &resolved);
+
+  return found;
+}
+
+bool
+ResolveGenericArgs::is_type_name (const CanonicalPath &path)
+{
+  NodeId resolved;
+  auto found = resolver->get_type_scope ().lookup (path, &resolved);
+
+  return found;
+}
+
 void
-ResolveGenericArgs::go (AST::GenericArgs &args)
+ResolveGenericArgs::disambiguate (AST::GenericArg &arg)
 {
-  for (auto &arg : args.get_generic_args ())
+  auto path = canonical_prefix.append (
+    CanonicalPath::new_seg (UNKNOWN_NODEID, arg.get_path ()));
+
+  auto is_type = is_type_name (path);
+  auto is_value = is_const_value_name (path);
+
+  // In case we cannot find anything, we resolve the ambiguity to a type.
+  // This causes the typechecker to error out properly and when necessary.
+  // But types also take priority over const values in the case of
+  // ambiguities, hence the weird control flow
+  if (is_type || (!is_type && !is_value))
+    arg = arg.disambiguate_to_type ();
+  else if (is_value)
+    arg = arg.disambiguate_to_const ();
+}
+
+void
+ResolveGenericArgs::resolve_disambiguated_generic (AST::GenericArg &arg)
+{
+  switch (arg.get_kind ())
     {
-      // FIXME: Arthur: Ugly hack while waiting for disambiguation
-      if (arg.get_kind () == AST::GenericArg::Kind::Either)
-	arg = arg.disambiguate_to_type ();
+    case AST::GenericArg::Kind::Const:
+      ResolveExpr::go (arg.get_expression ().get (), prefix, canonical_prefix);
+      break;
+    case AST::GenericArg::Kind::Type:
+      ResolveType::go (arg.get_type ().get ());
+      break;
+    default:
+      gcc_unreachable ();
+    }
+}
+void
+ResolveGenericArgs::go (AST::GenericArgs &generic_args)
+{
+  auto empty = CanonicalPath::create_empty ();
 
-      if (arg.get_kind () == AST::GenericArg::Kind::Type)
-	ResolveType::go (arg.get_type ().get ());
+  go (generic_args, empty, empty);
+}
+
+void
+ResolveGenericArgs::go (AST::GenericArgs &generic_args,
+			const CanonicalPath &prefix,
+			const CanonicalPath &canonical_prefix)
+{
+  auto resolver = ResolveGenericArgs (prefix, canonical_prefix);
+
+  for (auto &arg : generic_args.get_generic_args ())
+    {
+      if (arg.get_kind () == AST::GenericArg::Kind::Either)
+	resolver.disambiguate (arg);
 
-      // else...
-      // We need to use a switch instead
+      resolver.resolve_disambiguated_generic (arg);
     }
 }
 
diff --git a/gcc/rust/resolve/rust-ast-resolve-type.h b/gcc/rust/resolve/rust-ast-resolve-type.h
index 1679831..b57b513 100644
--- a/gcc/rust/resolve/rust-ast-resolve-type.h
+++ b/gcc/rust/resolve/rust-ast-resolve-type.h
@@ -254,6 +254,30 @@ class ResolveGenericArgs : public ResolverBase
 
 public:
   static void go (AST::GenericArgs &generic_args);
+  static void go (AST::GenericArgs &generic_args, const CanonicalPath &prefix,
+		  const CanonicalPath &canonical_prefix);
+
+private:
+  ResolveGenericArgs (const CanonicalPath &prefix,
+		      const CanonicalPath &canonical_prefix)
+    : ResolverBase (), prefix (prefix), canonical_prefix (canonical_prefix)
+  {}
+
+  bool is_type_name (const CanonicalPath &path);
+  bool is_const_value_name (const CanonicalPath &path);
+
+  /**
+   * Resolve a disambiguated generic arg
+   */
+  void disambiguate (AST::GenericArg &arg);
+
+  /**
+   * Resolve a disambiguated generic arg
+   */
+  void resolve_disambiguated_generic (AST::GenericArg &arg);
+
+  const CanonicalPath &prefix;
+  const CanonicalPath &canonical_prefix;
 };
 
 } // namespace Resolver
diff --git a/gcc/testsuite/rust/compile/const_generics_5.rs b/gcc/testsuite/rust/compile/const_generics_5.rs
new file mode 100644
index 0000000..5344e31
--- /dev/null
+++ b/gcc/testsuite/rust/compile/const_generics_5.rs
@@ -0,0 +1,12 @@
+struct Foo<const N: usize = { 14 }>;
+
+const M: usize = 15;
+type N = Foo<3>;
+
+fn main() {
+    let _: Foo<15> = Foo;
+    let _: Foo<{ M }> = Foo;
+    let _: Foo<M> = Foo;
+    // bogus error, but it means the above const generic gets disambiguated properly
+    let _: Foo<N> = Foo; // { dg-error "TypePath Foo<N> declares generic arguments but the type Foo{Foo {}} does not have any" }
+}