[gcc/devel/rust/master] resolver: Disambiguate generic args
Thomas Schwinge
tschwinge@gcc.gnu.org
Tue Jul 19 07:40:03 GMT 2022
https://gcc.gnu.org/g:d82201be88ce09c92db709573171c869d94b55fd
commit d82201be88ce09c92db709573171c869d94b55fd
Author: Arthur Cohen <arthur.cohen@embecosm.com>
Date: Tue Jul 5 18:53:05 2022 +0200
resolver: Disambiguate generic args
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.
Diff:
---
gcc/rust/expand/rust-attribute-visitor.cc | 41 +++++++----
gcc/rust/hir/rust-ast-lower-base.cc | 7 --
gcc/rust/hir/rust-ast-lower-type.h | 7 +-
gcc/rust/resolve/rust-ast-resolve-expr.cc | 2 +-
gcc/rust/resolve/rust-ast-resolve-type.cc | 98 +++++++++++++++++++-------
gcc/rust/resolve/rust-ast-resolve-type.h | 24 +++++++
gcc/testsuite/rust/compile/const_generics_5.rs | 12 ++++
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 749f7ebfed2..9332bb6ebfc 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 3db7dc312eb..e31b95b07bd 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 317fe851a88..46f765a3ea2 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 2553c854641..4cc4e26e3e9 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 04183a4bc61..a8931ce72c2 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 16798318227..b57b5138656 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 00000000000..5344e31a140
--- /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" }
+}
More information about the Gcc-cvs
mailing list