public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Thomas Schwinge <tschwinge@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] resolver: Disambiguate generic args Date: Tue, 19 Jul 2022 07:40:03 +0000 (GMT) [thread overview] Message-ID: <20220719074003.CC7893858285@sourceware.org> (raw) 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" } +}
reply other threads:[~2022-07-19 7:40 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220719074003.CC7893858285@sourceware.org \ --to=tschwinge@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).